Conversation
060194f to
dd4e6cc
Compare
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com> Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com> Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com> Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com> Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
…mantics Relocate audit log implementation to sdk/auditlog and add integrity, status mapping, idempotency conflict checks, and explicit policy hooks for 401/403/413/429 outcomes. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Split storage, store, identity, and status concerns into dedicated sdk/auditlog subpackages and keep root-level API wrappers/aliases so existing call sites continue to compile while improving readability. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
| type AuditLogger interface { | ||
| Emit(ctx context.Context, record AuditRecord) error | ||
| EmitWithResult(ctx context.Context, record AuditRecord) AuditEmitResult | ||
| Enabled(ctx context.Context, eventName string) bool |
There was a problem hiding this comment.
AuditLogger can't be disabled, when used it's always enabled!
| Enabled(ctx context.Context, eventName string) bool |
| } | ||
|
|
||
| type AuditRecordProcessor interface { | ||
| Enabled(ctx context.Context, param AuditEnabledParameters) bool |
There was a problem hiding this comment.
AuditRecords should ALWAYS be processed... so the AuditRecordProcessor is always enabled.
| Enabled(ctx context.Context, param AuditEnabledParameters) bool |
| auditAttrActor = "audit.actor" | ||
| auditAttrActorType = "audit.actor_type" |
There was a problem hiding this comment.
| auditAttrActor = "audit.actor" | |
| auditAttrActorType = "audit.actor_type" | |
| auditAttrActor = "audit.actor.id" | |
| auditAttrActorType = "audit.actor.type" |
| auditAttrActor = "audit.actor" | ||
| auditAttrActorType = "audit.actor_type" | ||
| auditAttrAction = "audit.action" | ||
| auditAttrResource = "audit.resource" |
There was a problem hiding this comment.
maybe we should split this into two:
| auditAttrResource = "audit.resource" | |
| auditAttrTargetID = "audit.target.id" | |
| auditAttrTargetType = "audit.target.type" |
| auditAttrSourceIP = "audit.source_ip" | ||
| auditAttrRecordID = "audit.record_id" |
There was a problem hiding this comment.
| auditAttrSourceIP = "audit.source_ip" | |
| auditAttrRecordID = "audit.record_id" | |
| auditAttrSourceID = "audit.source.id" | |
| auditAttrRecordID = "audit.record.id" |
| auditAttrHash = "audit.hash" | ||
| auditAttrSchemaVersion = "audit.schema_version" | ||
| auditAttrKeyID = "audit.key_id" | ||
| auditAttrSequenceNo = "audit.sequence_no" |
There was a problem hiding this comment.
| auditAttrSequenceNo = "audit.sequence_no" | |
| auditAttrSequenceNo = "audit.sequence.number" |
| auditAttrSignature = "audit.signature" | ||
| auditAttrHMAC = "audit.hmac" | ||
| auditAttrHash = "audit.hash" | ||
| auditAttrSchemaVersion = "audit.schema_version" |
There was a problem hiding this comment.
| auditAttrSchemaVersion = "audit.schema_version" | |
| auditAttrSchemaVersion = "audit.schema.version" |
| if record.Body().Kind() == log.KindEmpty { | ||
| return newAuditStatusError(AuditErrorInvalidRequest, "audit body is required", false, nil) | ||
| } |
There was a problem hiding this comment.
I'm not 100% sure if we ALWAYS require a body... for some scenarios the auditAttr* may be sufficient. Let's allow empty bodys for now.
| if record.Body().Kind() == log.KindEmpty { | |
| return newAuditStatusError(AuditErrorInvalidRequest, "audit body is required", false, nil) | |
| } | |
| // if record.Body().Kind() == log.KindEmpty { | |
| // return newAuditStatusError(AuditErrorInvalidRequest, "audit body is required", false, nil) | |
| // } |
| if record.AttributesLen() == 0 { | ||
| return newAuditStatusError(AuditErrorInvalidRequest, "audit attributes are required", false, nil) | ||
| } |
There was a problem hiding this comment.
When is this check executed? Before or after otelRecord.AddAttributes(auditAttrs...)?
| if record.SchemaVersion == "" { | ||
| return newAuditStatusError(AuditErrorInvalidRequest, "audit schema_version is required", false, nil) | ||
| } |
There was a problem hiding this comment.
should not be mandatory
| if record.SchemaVersion == "" { | |
| return newAuditStatusError(AuditErrorInvalidRequest, "audit schema_version is required", false, nil) | |
| } |
There was a problem hiding this comment.
Why are those two files: sdk/log/go.mod and sdk/log/go.sum removed?
| Priority int | ||
| } | ||
|
|
||
| type PriorityQueue []PriorityRecord |
There was a problem hiding this comment.
the longer I think about this, the more I believe it's not a good idea to use any PriorityQueue. Receivery might get very confused, when logs don't arrive in time sequence. Can we please stick for now with FIFO?
There was a problem hiding this comment.
if we wanna be really flexible, then we can also keep the PriorityQueue, but allow users to inject the comparing function (something which compares two log-records)
These files were deleted by mistake in the auditlog relocation commit. sdk/log remains a separate module; only sdk/auditlog was intended to be added as a new module. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
…alidation Remove Enabled from AuditLogger and AuditRecordProcessor, rename exported audit attribute keys to dotted form, auto-generate record IDs, relax body and schema validation, export queued records in FIFO order, and update tests and docs. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Introduce sdk/auditlog/stresstest with a mock OTLP HTTP receiver, shared harness, and end-to-end tests for eventual delivery, file-store crash recovery, retry limits, FIFO ordering, wait-on-export semantics, storage write modes, and concurrent emit. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Strengthen rejected-record validation by asserting rejected record IDs never reach the sink, raise default stress volume to 200k, and clean AUDIT_LOG_README content so it matches the current auditlog implementation. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Code reviewFound 6 issues (compliance with CLAUDE.md / otel-audit-logging spec): 1.
opentelemetry-go/sdk/auditlog/audit_logger_api.go Lines 203 to 221 in 0762a93 2.
opentelemetry-go/sdk/auditlog/audit_logger_api.go Lines 240 to 244 in 0762a93 3. Non-spec integrity attribute names — breaks interoperability with the Collector The SDK emits opentelemetry-go/sdk/auditlog/audit_logger_api.go Lines 52 to 59 in 0762a93 4. Records silently dropped when In opentelemetry-go/sdk/auditlog/audit_processor.go Lines 537 to 547 in 0762a93 5. No SDK observability metrics implemented The entire opentelemetry-go/sdk/auditlog/audit_processor.go Lines 519 to 548 in 0762a93 6. Default integrity config is deny-all — zero-config provider rejects every record
opentelemetry-go/sdk/auditlog/audit_integrity_config.go Lines 43 to 45 in 0762a93 🤖 Generated with Claude Code If this code review was useful, please react with 👍. Otherwise, react with 👎. |
| auditAttrSchemaVersion = "audit.schema.version" | ||
| auditAttrKeyID = "audit.key_id" | ||
| auditAttrSequenceNo = "audit.sequence.number" | ||
| auditAttrPrevHash = "audit.prev_hash" |
There was a problem hiding this comment.
| auditAttrPrevHash = "audit.prev_hash" | |
| auditAttrPrevHash = "audit.prev.hash" |
| targetID, _ := auditTargetFields(record) | ||
| if targetID == "" { | ||
| return newAuditStatusError(AuditErrorInvalidRequest, "audit target id is required", false, nil) | ||
| } |
There was a problem hiding this comment.
not sure if we'll have a target in all cases... e.g. user logs in - what's the target?
let's start with target being optional
| targetID, _ := auditTargetFields(record) | |
| if targetID == "" { | |
| return newAuditStatusError(AuditErrorInvalidRequest, "audit target id is required", false, nil) | |
| } |
Introduce go.opentelemetry.io/otel/audit with AuditReceipt and SdkAuditProvider. Add JCS HMAC integrity attributes, synchronous export by default, OTLP /v1/audit exporter, export receipts, and spec alignment tests. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Emit spec audit.records.* metrics, warn on NTP/timestamp skew, clone records in the processor queue, and treat OTLP partial success as export failure. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
…op errors. Make integrity proofs optional by default, export only spec-aligned integrity attributes, and surface hard errors to synchronous emit callers when retry budget is exhausted. Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
Co-authored-by: Hilmar Falkenberg <hilmar.falkenberg@sap.com> Signed-off-by: MJarmo <38920471+MJarmo@users.noreply.github.com>
Co-authored-by: Hilmar Falkenberg <hilmar.falkenberg@sap.com> Signed-off-by: MJarmo <38920471+MJarmo@users.noreply.github.com>
Co-authored-by: Hilmar Falkenberg <hilmar.falkenberg@sap.com> Signed-off-by: MJarmo <38920471+MJarmo@users.noreply.github.com>
Move integrity algorithm and certificate to resource attributes, validate UUID v4 record IDs, normalize field casing, omit severity on export, add SinkTimestampNanos to AuditReceipt, and use audit.integrity.value with IntegrityAlgorithm for all proofs (HMAC, hash, signature). Signed-off-by: MJarmo <michal.jarmolkiewicz@sap.com>
POC: Go auditlog SDK (file-backed audit logging + processing)
Summary
This PR introduces a proof-of-concept audit logging SDK for Go that provides file-based storage and processing of audit log records for use with OpenTelemetry. It implements a file-backed log sink, record serialization, and basic processing/consumption primitives to enable reliable, append-only audit logging with options for batch processing and rotation.
Why
What changed (high-level)
Design notes
Compatibility & Migration
Testing & validation
Unit tests for serialization, basic append/read operations, and rotation.
Please add CI integration and more end-to-end tests as the API stabilizes.