Skip to content

feat(observability): support header param while OTLP export#1805

Merged
qin-ctx merged 2 commits intovolcengine:mainfrom
baojun-zhang:Observer-enhance
Apr 29, 2026
Merged

feat(observability): support header param while OTLP export#1805
qin-ctx merged 2 commits intovolcengine:mainfrom
baojun-zhang:Observer-enhance

Conversation

@baojun-zhang
Copy link
Copy Markdown
Collaborator

@baojun-zhang baojun-zhang commented Apr 29, 2026

Description

feat(observability): support header param while OTLP export

Related Issue

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 not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • support header param while OTLP export

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

Additional Notes

#1805

Supports adding header parameters during OTLP transmission, suitable for scenarios requiring additional information (such as authentication SK/SK).
Configuration as follows

  "traces": {
            "enabled": false,
            "protocol": "grpc",
            "tls": {
                "insecure": false
            },
            "endpoint": "localhost:4317",
            "service_name": "openviking-server",
            "headers": {
              "xxx-header-1" :"xxx-value-1",
              "xxx-header-2" :"xxx-value-2"
            }
        },

Example -- Integrating into the Volcano Engine log service system requires adding information such as topic/ak/sk.

{
    "traces": {
        "enabled": true,
        "protocol": "grpc",
        "tls": {
            "insecure": false
        },
        "endpoint": "tls-cn-beijing.volces.com:4317",
        "service_name": "openviking-server-test0429",
        "headers": {
            "x-tls-otel-tracetopic": "your-topic",
            "x-tls-otel-ak": "your-ak",
            "x-tls-otel-sk": "your-sk",
            "x-tls-otel-region": "cn-beijing"
        }
    }
}

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 85
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Fix observability context reset semantics by cloning

Relevant files:

  • openviking/observability/context.py

Sub-PR theme: Add OTLP headers support for metrics, traces, and logs

Relevant files:

  • openviking/metrics/exporters/otel.py
  • openviking/metrics/global_api.py
  • openviking/telemetry/tracer.py
  • openviking_cli/utils/logger.py
  • openviking/server/config.py
  • tests/metrics/core/test_exporter.py
  • tests/server/test_prometheus_metrics.py
  • tests/test_server_config_loader.py
  • tests/test_telemetry_runtime.py
  • docs/en/concepts/12-metrics.md
  • docs/en/guides/05-observability.md
  • docs/zh/concepts/12-metrics.md
  • docs/zh/guides/05-observability.md
  • examples/ov.conf.example

⚡ Recommended focus areas for review

Redundant code in gRPC exporter initialization

The try/except block for gRPC trace exporter initializes the exporter with the same arguments (endpoint and headers) in both branches, which serves no purpose and can be removed.

try:
    trace_exporter = OTLPGrpcSpanExporter(
        endpoint=endpoint,
        insecure=insecure,
        headers=normalized_headers,
    )
except TypeError:
    trace_exporter = OTLPGrpcSpanExporter(
        endpoint=endpoint,
        headers=normalized_headers,
    )
Redundant code in gRPC exporter initialization

The try/except block for gRPC log exporter initializes the exporter with the same arguments (endpoint and headers) in both branches, which serves no purpose and can be removed.

try:
    otlp_exporter = OTLPGrpcLogExporter(
        endpoint=endpoint,
        insecure=insecure,
        headers=normalized_headers,
    )
except TypeError:
    otlp_exporter = OTLPGrpcLogExporter(
        endpoint=endpoint,
        headers=normalized_headers,
    )

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove conditional gRPC metadata branch

Simplify the gRPC export call by always passing the metadata list (even empty)
instead of using a conditional branch. gRPC stub methods handle empty metadata
gracefully.

openviking/metrics/exporters/otel.py [616-624]

-metadata = list(self._headers.items()) if self._headers else None
-if metadata:
-    self._grpc_stub.Export(
-        request,
-        timeout=self._export_timeout_seconds,
-        metadata=metadata,
-    )
-else:
-    self._grpc_stub.Export(request, timeout=self._export_timeout_seconds)
+metadata = list(self._headers.items())
+self._grpc_stub.Export(
+    request,
+    timeout=self._export_timeout_seconds,
+    metadata=metadata,
+)
Suggestion importance[1-10]: 5

__

Why: Simplifies code by always passing metadata (even empty) to the gRPC Export call, which is safe and reduces unnecessary branching. This is a minor readability/maintainability improvement.

Low

Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding OTLP header support across traces, logs, and metrics.

I found one blocking issue: the new config accepts and documents mixed-case header keys, but the gRPC export paths forward those keys as metadata, and grpcio rejects uppercase metadata keys at call time. Please normalize or validate header keys before they reach gRPC metadata.

Non-blocking notes:

  • This PR does not link a related issue. If this is for a specific backend integration requirement, please add the issue or design context for traceability.
  • The current docs build is failing in docs/zh/api/99-api-doc-writing-guide.md:47. That appears unrelated to this PR, but the PR is still blocked until CI is green or the baseline is clarified.

endpoint: str = "localhost:4317" # gRPC default: 4317; HTTP default: 4318
service_name: str = "openviking-server"
export_interval_ms: int = 10000
headers: Dict[str, str] = Field(default_factory=dict)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Bug] (blocking)

headers currently accepts arbitrary keys and the gRPC paths pass them through as metadata. gRPC Python requires metadata keys to be lowercase; for example, the documented/tested X-ByteAPM-AppKey form is rejected by grpcio with ValueError: metadata was invalid before the request is sent.

This means users following the new protocol="grpc" examples with X-* auth headers will see OTLP exports fail continuously. Please either normalize keys before passing them to gRPC metadata, or validate config and reject non-lowercase metadata keys. The tests should also cover the real grpcio metadata constraint, not only a fake stub that accepts uppercase keys.

@qin-ctx qin-ctx merged commit d7fdb48 into volcengine:main Apr 29, 2026
7 of 8 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants