Skip to content

fix(auditlog_ng): infer OTLP protocol from endpoint when env unset - #411

Open
jeanscherf wants to merge 1 commit into
mainfrom
fix/audit-protocol-from-endpoint
Open

jeanscherf wants to merge 1 commit into
mainfrom
fix/audit-protocol-from-endpoint

Conversation

@jeanscherf

Copy link
Copy Markdown
Member

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

The Audit Log NG client picked its OTLP transport from OTEL_EXPORTER_OTLP_PROTOCOL, defaulting to gRPC when that variable was unset (_create_log_exporter in src/sap_cloud_sdk/core/auditlog_ng/client.py). In common deployments that variable is never injected, so an HTTP audit endpoint (for example http://host:4318/v1/logs) was always dialed with a gRPC exporter and delivery failed.

This derives the protocol from the resolved config.endpoint instead. New helper _infer_protocol_from_endpoint reads the endpoint shape (an http(s) URL with a path means http/protobuf; a bare host:port means grpc), and _resolve_protocol applies it: an explicit OTEL_EXPORTER_OTLP_PROTOCOL still wins when set, inference is used when it is unset, and gRPC remains the fallback when both the env and the shape are inconclusive.

Related Issue

N/A — no public issue reference.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • Dependency update

How to Test

  1. Unset OTEL_EXPORTER_OTLP_PROTOCOL, build AuditClient with endpoint="http://host:4318/v1/logs", and confirm an HTTPLogExporter is constructed (previously a GRPCLogExporter was used).
  2. Unset the env and build with endpoint="audit.example.com:443"; confirm a GRPCLogExporter is constructed.
  3. Set OTEL_EXPORTER_OTLP_PROTOCOL=grpc with an HTTP endpoint and confirm gRPC is used (explicit override wins).
  4. Run uv run pytest tests/core/unit/auditlog_ng/unit/test_client.py -k "Protocol or Infer or Resolve".

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Additional Notes

Design note: an endpoint with no path resolves to gRPC even when it carries an http/https scheme, because a path is the only positive HTTP signal and OTLP/gRPC endpoints legitimately use an http(s) scheme to indicate (in)secure transport. The rare "HTTP base endpoint without /v1/logs" case is covered by setting OTEL_EXPORTER_OTLP_PROTOCOL explicitly. This behavior is pinned by a test.

Unit tests cover every branch of both helpers (http/grpc, with and without port, scheme vs no scheme, no host, empty/None, and the env precedence/fallback paths).

ruff format also normalized a few pre-existing long lines in tests/core/unit/auditlog_ng/unit/test_client.py (imports, a helper signature, one assertion) that were touched by the required pre-commit hook.

The audit client chose its OTLP protocol from OTEL_EXPORTER_OTLP_PROTOCOL,
defaulting to gRPC when it was unset. The platform instrumentation never
injects that variable, so an HTTP audit destination (http://host:4318/v1/logs)
was always dialed with a gRPC exporter, breaking delivery.

Derive the protocol from the resolved endpoint shape instead: an http(s) URL
with a path -> http/protobuf, a bare host:port -> grpc. An explicit
OTEL_EXPORTER_OTLP_PROTOCOL still wins when set, and gRPC remains the fallback
when both the env and the endpoint shape are inconclusive.

Add unit tests covering every branch of the shape inference and the resolver.
@jeanscherf
jeanscherf marked this pull request as ready for review October 19, 2026 14:08
@jeanscherf
jeanscherf requested a review from a team as a code owner October 19, 2026 14:08

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant