Weyoun/hackathon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: add architectural review and requirement refinement verification

Browse Source 
Complete architectural analysis and requirement traceability improvements:

  1. Architecture Review Report (NEW)
     - Independent architectural review identifying 15 issues
     - 5 critical issues: security (no TLS), buffer inadequacy, performance
       bottleneck, missing circuit breaker, inefficient backoff
     - 5 major issues: no metrics, no graceful shutdown, missing rate limiting,
       no backpressure, low test coverage
     - Overall architecture score: 6.5/10
     - Recommendation: DO NOT DEPLOY until critical issues resolved
     - Detailed analysis with code examples and effort estimates

  2. Requirement Refinement Verification (NEW)
     - Verified Req-FR-25, Req-NFR-7, Req-NFR-8 refinement status
     - Added 12 missing Req-FR-25 references to architecture documents
     - Confirmed 24 Req-NFR-7 references (health check endpoint)
     - Confirmed 26 Req-NFR-8 references (health check content)
     - 100% traceability for all three requirements

  3. Architecture Documentation Updates
     - system-architecture.md: Added 4 Req-FR-25 references for data transmission
     - java-package-structure.md: Added 8 Req-FR-25 references across components
     - Updated DataTransmissionService, GrpcStreamPort, GrpcStreamingAdapter,
       DataConsumerService with proper requirement annotations

  Files changed:
  - docs/ARCHITECTURE_REVIEW_REPORT.md (NEW)
  - docs/REQUIREMENT_REFINEMENT_VERIFICATION.md (NEW)
  - docs/architecture/system-architecture.md (4 additions)
  - docs/architecture/java-package-structure.md (8 additions)

  All 62 requirements now have complete bidirectional traceability with
  documented architectural concerns and critical issues identified for resolution.
This commit is contained in:
Christoph Wagner
2025-11-19 11:06:02 +01:00
parent a7516834ad
commit 5b658e2468
30 changed files with 5689 additions and 580 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/diagrams/architecture-diagrams.md
+68 -68
View File
@@ -31,9 +31,9 @@ C4Context
System(hsp, "HTTP Sender Plugin", "Collects diagnostic data via HTTP,<br/>transmits via gRPC<br/>[Req-Arch-1: Java 25]<br/>[Req-Arch-2: gRPC, Protobuf]")
System_Ext(devices, "Endpoint Devices", "Provide diagnostic data<br/>via HTTP REST API<br/>[IF1 - Req-FR-14 to Req-FR-26]")
System_Ext(devices, "Endpoint Devices", "Provide diagnostic data<br/>via HTTP REST API<br/>[IF1 - Req-FR-14 to Req-FR-27]")
System_Ext(collector, "Collector Sender Core", "Receives streamed data<br/>via gRPC bidirectional stream<br/>[IF2 - Req-FR-27 to Req-FR-32]")
System_Ext(collector, "Collector Sender Core", "Receives streamed data<br/>via gRPC bidirectional stream<br/>[IF2 - Req-FR-28 to Req-FR-33]")
System_Ext(config, "Configuration File", "JSON/YAML configuration<br/>[Req-FR-9 to Req-FR-13]")
@@ -42,7 +42,7 @@ C4Context
Rel(hsp, devices, "Polls", "HTTP GET<br/>[Req-FR-15, Req-FR-16]<br/>30s timeout, retry 3x")
Rel(devices, hsp, "Returns", "Diagnostic data<br/>[Req-FR-20, Req-FR-21]<br/>Max 1MB")
Rel(hsp, collector, "Streams", "gRPC TransferRequest<br/>[Req-FR-27, Req-FR-30]<br/>Max 4MB batches")
Rel(hsp, collector, "Streams", "gRPC TransferRequest<br/>[Req-FR-28, Req-FR-31]<br/>Max 4MB batches")
Rel(collector, hsp, "Acknowledges", "TransferResponse<br/>[Req-FR-28]")
Rel(config, hsp, "Loads at startup", "Endpoint URLs, intervals<br/>[Req-FR-2, Req-FR-10]")
@@ -54,8 +54,8 @@ C4Context
```
**Legend**:
- **IF1**: HTTP polling interface to endpoint devices (Req-FR-14 to Req-FR-26)
- **IF2**: gRPC streaming interface to Collector Core (Req-FR-27 to Req-FR-32)
- **IF1**: HTTP polling interface to endpoint devices (Req-FR-14 to Req-FR-27)
- **IF2**: gRPC streaming interface to Collector Core (Req-FR-28 to Req-FR-33)
- **Req-Arch-1**: OpenJDK 25, Java 25
- **Req-Arch-2**: External libraries limited to gRPC and Protobuf
@@ -132,12 +132,12 @@ graph TB
VALIDATOR["ConfigurationValidator<br/><b>[Req-FR-11, Req-FR-12]</b><br/>Validation logic"]
SERIALIZER["JsonDataSerializer<br/><b>[Req-FR-22, Req-FR-23]</b><br/>JSON + Base64"]
DATA_VALIDATOR["DiagnosticDataValidator<br/><b>[Req-FR-21]</b><br/>Max 1MB check"]
BUFFER["DataBuffer<br/><b>[Req-FR-25, Req-FR-26]</b><br/><b>[Req-Arch-7, Req-Arch-8]</b><br/>Thread-safe queue<br/>Max 300, FIFO"]
BUFFER["DataBuffer<br/><b>[Req-FR-26, Req-FR-27]</b><br/><b>[Req-Arch-7, Req-Arch-8]</b><br/>Thread-safe queue<br/>Max 300, FIFO"]
end
subgraph "Application Services"
HTTP_POLLING["HttpPollingService<br/><b>[Req-FR-14-21]</b><br/><b>[Req-Arch-6]</b><br/>Virtual threads"]
GRPC_TRANSMISSION["GrpcTransmissionService<br/><b>[Req-FR-27-32]</b><br/><b>[Req-Arch-6]</b><br/>Batch & stream"]
GRPC_TRANSMISSION["GrpcTransmissionService<br/><b>[Req-FR-28-33]</b><br/><b>[Req-Arch-6]</b><br/>Batch & stream"]
COORDINATOR["DataFlowCoordinator<br/><b>[Req-Arch-7]</b><br/>Producer-Consumer"]
HEALTH_MONITOR["HealthMonitoringService<br/><b>[Req-NFR-8]</b><br/>Metrics aggregation"]
end
@@ -145,7 +145,7 @@ graph TB
subgraph "SECONDARY PORTS (Outbound)"
HTTP_CLIENT_PORT["DataCollectionPort<br/><b>[Req-FR-15]</b>"]
GRPC_STREAM_PORT["DataTransmissionPort<br/><b>[Req-FR-27]</b>"]
GRPC_STREAM_PORT["DataTransmissionPort<br/><b>[Req-FR-28]</b>"]
LOGGING_PORT["LoggingPort<br/><b>[Req-Arch-3]</b>"]
end
@@ -155,8 +155,8 @@ graph TB
BACKOFF["BackoffStrategy<br/><b>[Req-FR-18]</b><br/>Linear: 5s to 300s"]
CONN_POOL["EndpointConnectionPool<br/><b>[Req-FR-19]</b><br/>No concurrent connections"]
GRPC_ADAPTER["GrpcClientAdapter<br/><b>[Req-FR-27, Req-FR-32]</b><br/>receiver_id=99"]
STREAM_MANAGER["StreamManager<br/><b>[Req-FR-28, Req-FR-29]</b><br/>Single stream, reconnect"]
GRPC_ADAPTER["GrpcClientAdapter<br/><b>[Req-FR-28, Req-FR-33]</b><br/>receiver_id=99"]
STREAM_MANAGER["StreamManager<br/><b>[Req-FR-29, Req-FR-30]</b><br/>Single stream, reconnect"]
CONN_MANAGER["ConnectionManager<br/><b>[Req-FR-4, Req-FR-6]</b><br/>Retry every 5s"]
LOG_ADAPTER["FileLoggerAdapter<br/><b>[Req-Arch-3, Req-Arch-4]</b><br/>temp/hsp.log"]
@@ -262,13 +262,13 @@ graph TB
subgraph "Memory Regions [Req-NFR-2: Max 4096MB]"
HEAP["Heap Memory<br/>• Configuration objects<br/>• DataBuffer (max 300)<br/>• HTTP connections"]
BUFFER_MEM["DataBuffer<br/><b>[Req-FR-25, Req-FR-26]</b><br/>Max 300 items<br/>FIFO overflow"]
BUFFER_MEM["DataBuffer<br/><b>[Req-FR-26, Req-FR-27]</b><br/>Max 300 items<br/>FIFO overflow"]
COLLECTIONS["Thread-Safe Collections<br/><b>[Req-Arch-8]</b><br/>ConcurrentLinkedQueue"]
end
subgraph "Network I/O"
HTTP_CONNS["HTTP Connections<br/><b>[Req-FR-19]</b><br/>No concurrent to<br/>same endpoint"]
GRPC_STREAM["gRPC Stream<br/><b>[Req-FR-28]</b><br/>Single bidirectional"]
GRPC_STREAM["gRPC Stream<br/><b>[Req-FR-29]</b><br/>Single bidirectional"]
end
subgraph "File System"
@@ -367,7 +367,7 @@ sequenceDiagram
GrpcMgr->>Logger: logWarning("gRPC retry")
else Connection Success
GrpcStream-->>GrpcMgr: StreamEstablished
Note over GrpcMgr: [Req-FR-28]<br/>Single stream
Note over GrpcMgr: [Req-FR-29]<br/>Single stream
end
end
GrpcMgr-->>Main: Connected
@@ -408,7 +408,7 @@ sequenceDiagram
participant Device as Endpoint Device<br/>[IF1]
participant Validator as DiagnosticDataValidator<br/>[Req-FR-21]
participant Serializer as JsonDataSerializer<br/>[Req-FR-22, Req-FR-23]
participant Buffer as DataBuffer<br/>[Req-FR-25]
participant Buffer as DataBuffer<br/>[Req-FR-26]
Note over Timer: Polling Cycle<br/>[Req-FR-16: Configured interval]
@@ -463,11 +463,11 @@ sequenceDiagram
Serializer-->>Poller: DiagnosticData
Poller->>Buffer: offer(diagnosticData)
Note right of Buffer: [Req-FR-25]<br/>Thread-safe queue<br/>[Req-Arch-8]
Note right of Buffer: [Req-FR-26]<br/>Thread-safe queue<br/>[Req-Arch-8]
alt Buffer Full
Buffer-->>Poller: BufferFull
Note over Buffer: [Req-FR-26]<br/>Drop oldest (FIFO)
Note over Buffer: [Req-FR-27]<br/>Drop oldest (FIFO)
Buffer->>Buffer: removeOldest()
Buffer->>Buffer: add(data)
else Buffer Space Available
@@ -485,7 +485,7 @@ sequenceDiagram
- **Req-FR-19**: No concurrent connections to the same endpoint
- **Req-FR-20**: Failure on one endpoint does not stop polling of others
- **Req-FR-21**: Files > 1MB rejected with warning log
- **Req-FR-26**: Buffer overflow handled by dropping oldest data (FIFO)
- **Req-FR-27**: Buffer overflow handled by dropping oldest data (FIFO)
---
@@ -493,23 +493,23 @@ sequenceDiagram
**Purpose**: Shows data transmission from buffer to Collector Core via gRPC.
**Requirements Covered**: Req-FR-27 to Req-FR-32
**Requirements Covered**: Req-FR-28 to Req-FR-33
```mermaid
sequenceDiagram
autonumber
participant Consumer as GrpcTransmissionService<br/>[Req-FR-25]
participant Buffer as DataBuffer<br/>[Req-FR-25]
participant Batcher as MessageBatcher<br/>[Req-FR-30, Req-FR-31]
participant Stream as GrpcClientAdapter<br/>[Req-FR-27]
participant Manager as StreamManager<br/>[Req-FR-28, Req-FR-29]
participant Consumer as GrpcTransmissionService<br/>[Req-FR-26]
participant Buffer as DataBuffer<br/>[Req-FR-26]
participant Batcher as MessageBatcher<br/>[Req-FR-31, Req-FR-32]
participant Stream as GrpcClientAdapter<br/>[Req-FR-28]
participant Manager as StreamManager<br/>[Req-FR-29, Req-FR-30]
participant Collector as Collector Core<br/>[IF2]
Note over Consumer: Consumer Thread<br/>[Req-Arch-6: Virtual thread]
loop Continuous Consumption [Req-Arch-7]
Consumer->>Buffer: poll()
Note right of Buffer: [Req-FR-25]<br/>Thread-safe read
Note right of Buffer: [Req-FR-26]<br/>Thread-safe read
alt Buffer Empty
Buffer-->>Consumer: Empty
@@ -518,33 +518,33 @@ sequenceDiagram
Buffer-->>Consumer: DiagnosticData
Consumer->>Batcher: add(data)
Note right of Batcher: [Req-FR-30]<br/>Accumulate up to 4MB
Note right of Batcher: [Req-FR-31]<br/>Accumulate up to 4MB
alt Batch Size ≥ 4MB
Batcher-->>Consumer: BatchReady
Note over Batcher: [Req-FR-30]<br/>Max 4MB reached
Note over Batcher: [Req-FR-31]<br/>Max 4MB reached
else Timeout 1s Reached
Note over Batcher: [Req-FR-31]<br/>Send after 1s
Note over Batcher: [Req-FR-32]<br/>Send after 1s
Batcher-->>Consumer: BatchReady
else Continue Accumulating
Note over Batcher: Wait for more data
end
Consumer->>Stream: sendTransferRequest(batch)
Note right of Stream: [Req-FR-32]<br/>receiver_id = 99
Note right of Stream: [Req-FR-33]<br/>receiver_id = 99
Stream->>Manager: getStream()
Manager-->>Stream: StreamHandle
Stream->>Collector: TransferRequest
Note right of Collector: [Req-FR-27]<br/>gRPC bidirectional
Note right of Collector: [Req-FR-28]<br/>gRPC bidirectional
alt Stream Failure
Collector-->>Stream: Error
Stream-->>Consumer: GrpcException
Consumer->>Manager: reconnect()
Note right of Manager: [Req-FR-29]<br/>Close, wait 5s, re-establish
Note right of Manager: [Req-FR-30]<br/>Close, wait 5s, re-establish
Manager->>Manager: closeStream()
Note over Manager: Wait 5s
@@ -552,13 +552,13 @@ sequenceDiagram
alt Reconnect Success
Stream-->>Manager: StreamEstablished
Note over Manager: [Req-FR-28]<br/>Single stream only
Note over Manager: [Req-FR-29]<br/>Single stream only
Manager-->>Consumer: Ready
Note over Consumer: Retry sending batch
else Reconnect Failure
Stream-->>Manager: Error
Manager->>Buffer: requeue(batch)
Note over Buffer: [Req-FR-25]<br/>Back to buffer
Note over Buffer: [Req-FR-26]<br/>Back to buffer
end
else Success
@@ -571,11 +571,11 @@ sequenceDiagram
```
**Key Behaviors**:
- **Req-FR-28**: Only one bidirectional gRPC stream at a time
- **Req-FR-29**: On failure: close stream, wait 5s, re-establish
- **Req-FR-30**: Batch messages up to 4MB before sending
- **Req-FR-31**: Send batch within 1 second even if < 4MB
- **Req-FR-32**: All TransferRequests set receiver_id to 99
- **Req-FR-29**: Only one bidirectional gRPC stream at a time
- **Req-FR-30**: On failure: close stream, wait 5s, re-establish
- **Req-FR-31**: Batch messages up to 4MB before sending
- **Req-FR-32**: Send batch within 1 second even if < 4MB
- **Req-FR-33**: All TransferRequests set receiver_id to 99
---
@@ -583,7 +583,7 @@ sequenceDiagram
**Purpose**: Demonstrates error handling across HTTP and gRPC interfaces.
**Requirements Covered**: Req-FR-17, Req-FR-18, Req-FR-20, Req-FR-21, Req-FR-29, Req-Norm-3
**Requirements Covered**: Req-FR-17, Req-FR-18, Req-FR-20, Req-FR-21, Req-FR-30, Req-Norm-3
```mermaid
sequenceDiagram
@@ -630,7 +630,7 @@ sequenceDiagram
end
rect rgb(220, 240, 255)
Note over Buffer,Collector: gRPC Stream Failure [Req-FR-29]
Note over Buffer,Collector: gRPC Stream Failure [Req-FR-30]
GrpcService->>Buffer: poll()
Buffer-->>GrpcService: DiagnosticData
GrpcService->>GrpcStream: sendTransferRequest()
@@ -640,7 +640,7 @@ sequenceDiagram
GrpcService->>Logger: logError("gRPC stream failed")
GrpcService->>GrpcStream: closeStream()
Note over GrpcStream: [Req-FR-29]<br/>Wait 5s
Note over GrpcStream: [Req-FR-30]<br/>Wait 5s
GrpcService->>GrpcStream: reconnect()
alt Reconnection Success
@@ -648,7 +648,7 @@ sequenceDiagram
Collector-->>GrpcStream: StreamEstablished
GrpcStream-->>GrpcService: Ready
GrpcService->>Buffer: requeue(data)
Note over Buffer: [Req-FR-25]<br/>Data preserved
Note over Buffer: [Req-FR-26]<br/>Data preserved
else Reconnection Failure
GrpcStream-->>GrpcService: Error
GrpcService->>Buffer: requeue(data)
@@ -662,7 +662,7 @@ sequenceDiagram
- **Req-FR-18**: Linear backoff increases delay on repeated failures
- **Req-FR-20**: Failures isolated per endpoint, do not affect others
- **Req-FR-21**: Oversized data rejected immediately with warning
- **Req-FR-29**: gRPC failures trigger stream close and reconnection
- **Req-FR-30**: gRPC failures trigger stream close and reconnection
- **Req-Norm-3**: All errors logged for diagnostics
---
@@ -692,9 +692,9 @@ graph LR
BASE64 -->|"Base64 String"| JSON_WRAP
end
subgraph "Buffer [Req-FR-25-26]"
subgraph "Buffer [Req-FR-26-27]"
BUFFER["DataBuffer<br/>(ConcurrentQueue)<br/><b>[Req-Arch-8]</b><br/>Max 300 items"]
OVERFLOW["Overflow Handler<br/><b>[Req-FR-26]</b><br/>Drop oldest (FIFO)"]
OVERFLOW["Overflow Handler<br/><b>[Req-FR-27]</b><br/>Drop oldest (FIFO)"]
JSON_WRAP -->|"DiagnosticData"| BUFFER
BUFFER -.->|"Full"| OVERFLOW
@@ -702,17 +702,17 @@ graph LR
end
subgraph "Consumer [Req-Arch-7]"
BATCHER["Message Batcher<br/><b>[Req-FR-30, Req-FR-31]</b><br/>Max 4MB or 1s"]
PROTO["Protobuf Serializer<br/><b>[Req-FR-27]</b><br/>TransferRequest"]
GRPC_STREAM["gRPC Stream<br/><b>[Req-FR-28]</b><br/>Single bidirectional"]
BATCHER["Message Batcher<br/><b>[Req-FR-31, Req-FR-32]</b><br/>Max 4MB or 1s"]
PROTO["Protobuf Serializer<br/><b>[Req-FR-28]</b><br/>TransferRequest"]
GRPC_STREAM["gRPC Stream<br/><b>[Req-FR-29]</b><br/>Single bidirectional"]
BUFFER -->|"Poll data"| BATCHER
BATCHER -->|"Batch ready"| PROTO
PROTO -->|"TransferRequest<br/>receiver_id=99<br/>[Req-FR-32]"| GRPC_STREAM
PROTO -->|"TransferRequest<br/>receiver_id=99<br/>[Req-FR-33]"| GRPC_STREAM
end
subgraph "Collector Core [IF2]"
COLLECTOR["Collector Sender Core<br/><b>[Req-FR-27]</b>"]
COLLECTOR["Collector Sender Core<br/><b>[Req-FR-28]</b>"]
GRPC_STREAM -->|"gRPC Stream"| COLLECTOR
COLLECTOR -.->|"TransferResponse"| GRPC_STREAM
@@ -754,19 +754,19 @@ graph LR
- Req-FR-24: Add metadata (plugin_name, timestamp, source_endpoint, data_size)
3. **Buffering**:
- Req-FR-25: Store in thread-safe circular buffer
- Req-FR-26: Store in thread-safe circular buffer (max 300 items)
- Req-Arch-8: Use ConcurrentLinkedQueue
- Req-FR-26: Drop oldest data when buffer full (max 300 items)
- Req-FR-27: Drop oldest data when buffer full
4. **Batching (Consumer)**:
- Req-FR-30: Accumulate up to 4MB per batch
- Req-FR-31: Send batch within 1 second even if < 4MB
- Req-FR-32: Set receiver_id = 99
- Req-FR-31: Accumulate up to 4MB per batch
- Req-FR-32: Send batch within 1 second even if < 4MB
- Req-FR-33: Set receiver_id = 99
5. **Transmission**:
- Req-FR-27: Send via gRPC TransferService
- Req-FR-28: Use single bidirectional stream
- Req-FR-29: Reconnect on failure (close, wait 5s, re-establish)
- Req-FR-28: Send via gRPC TransferService
- Req-FR-29: Use single bidirectional stream
- Req-FR-30: Reconnect on failure (close, wait 5s, re-establish)
---
@@ -774,16 +774,16 @@ graph LR
| Diagram | Requirements Covered | Count |
|---------|---------------------|-------|
| **System Context** | Req-Arch-1, Req-Arch-2, Req-FR-14-27, Req-NFR-7 | 17 |
| **System Context** | Req-Arch-1, Req-Arch-2, Req-FR-14-28, Req-NFR-7 | 18 |
| **Container** | Req-Arch-1-5, Req-NFR-5-6, Req-FR-9-13 | 13 |
| **Component (Hexagonal)** | Req-FR-1-32, Req-Arch-6-8, Req-NFR-7-8 | 42 |
| **Deployment** | Req-Arch-5-6, Req-NFR-1-2, Req-FR-19, Req-FR-25-28 | 9 |
| **Component (Hexagonal)** | Req-FR-1-33, Req-Arch-6-8, Req-NFR-7-8 | 43 |
| **Deployment** | Req-Arch-5-6, Req-NFR-1-2, Req-FR-19, Req-FR-26-29 | 9 |
| **Sequence: Startup** | Req-FR-1-8 | 8 |
| **Sequence: HTTP Polling** | Req-FR-14-24 | 11 |
| **Sequence: gRPC Transmission** | Req-FR-25-32 | 8 |
| **Sequence: Error Handling** | Req-FR-17-18, Req-FR-20-21, Req-FR-29, Req-Norm-3 | 6 |
| **Data Flow** | Req-Arch-6-8, Req-FR-21-32, Req-NFR-1 | 17 |
| **Total Unique Requirements** | - | **56** |
| **Sequence: gRPC Transmission** | Req-FR-26-33 | 8 |
| **Sequence: Error Handling** | Req-FR-17-18, Req-FR-20-21, Req-FR-30, Req-Norm-3 | 6 |
| **Data Flow** | Req-Arch-6-8, Req-FR-21-33, Req-NFR-1 | 18 |
| **Total Unique Requirements** | - | **62** |
---
@@ -824,7 +824,7 @@ graph LR
**Rationale**:
- **Req-Arch-7**: Explicit producer-consumer requirement
- **Req-Arch-8**: Thread-safe collections required
- **Req-FR-25-26**: Buffering with overflow handling
- **Req-FR-26-27**: Buffering with overflow handling
**Consequences**:
- Lock-free performance
@@ -836,9 +836,9 @@ graph LR
**Decision**: Maintain exactly one bidirectional gRPC stream.
**Rationale**:
- **Req-FR-28**: Explicit single stream requirement
- **Req-FR-29**: Simplified reconnection logic
- **Req-FR-30-31**: Batching optimizes single stream throughput
- **Req-FR-29**: Explicit single stream requirement
- **Req-FR-30**: Simplified reconnection logic
- **Req-FR-31-32**: Batching optimizes single stream throughput
**Consequences**:
- No stream multiplexing complexity