-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add headers to OTEL config #333
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes add support for specifying custom headers in the OpenTelemetry metrics and traces exporters. New flags for headers have been introduced in both CLI files, and the header values are parsed into maps. The configuration structures (OTLPConfig) in both modules now include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant FXModule
participant Config
participant ExporterModule
User->>CLI: Provide header flags (key=value pairs)
CLI->>FXModule: Parse custom header flags
FXModule->>Config: Build OTLPConfig with headers map
Config->>ExporterModule: Pass configuration data
ExporterModule->>ExporterModule: Conditionally add gRPC/HTTP header options
ExporterModule-->>User: Exporter setup complete with custom headers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
otlp/otlpmetrics/cli.go
(4 hunks)otlp/otlpmetrics/module.go
(3 hunks)otlp/otlptraces/cli.go
(4 hunks)otlp/otlptraces/traces.go
(3 hunks)
🔇 Additional comments (15)
otlp/otlpmetrics/module.go (3)
45-45
: LGTM - Adding Headers field to OTLPConfig structThe addition of the
Headers
field to theOTLPConfig
struct is a clean way to support custom headers in the OTLP configuration.
136-140
: LGTM - Header support for gRPC modeThe implementation correctly adds the headers option to the gRPC exporter when headers are provided.
156-160
: LGTM - Header support for HTTP modeThe implementation correctly adds the headers option to the HTTP exporter when headers are provided.
otlp/otlptraces/traces.go (3)
36-36
: LGTM - Adding Headers field to OTLPConfig structThe addition of the
Headers
field to theOTLPConfig
struct is consistent with the metrics module implementation.
113-117
: LGTM - Header support for gRPC modeThe implementation correctly adds the headers option to the gRPC exporter when headers are provided.
129-133
: LGTM - Header support for HTTP modeThe implementation correctly adds the headers option to the HTTP exporter when headers are provided.
otlp/otlptraces/cli.go (5)
4-4
: LGTM - Added strings importThe strings package is correctly imported to support the header parsing functionality.
21-21
: LGTM - New constant for OTLP headers flagThe constant is well-named and maintains consistency with the existing constants.
35-35
: LGTM - Added new flag for OTLP headersThe StringSlice flag type is appropriate for handling multiple headers.
54-62
: LGTM - Header parsing implementationThe header parsing logic correctly splits each header by the first '=' character and only adds entries with both key and value to the map.
68-68
: LGTM - Added Headers field to OTLPConfig instantiationThe headers map is correctly passed to the OTLPConfig struct.
otlp/otlpmetrics/cli.go (4)
4-4
: LGTM - Added strings importThe strings package is correctly imported to support the header parsing functionality.
23-23
: LGTM - New constant for OTLP headers flagThe constant is well-named and maintains consistency with the existing constants.
36-36
: LGTM - Added new flag for OTLP headersThe StringSlice flag type is appropriate for handling multiple headers.
63-63
: LGTM - Added Headers field to OTLPConfig instantiationThe headers map is correctly passed to the OTLPConfig struct.
otelMetricsExporterOTLPHeaders, _ := cmd.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag) | ||
headersMap := make(map[string]string) | ||
for _, header := range otelMetricsExporterOTLPHeaders { | ||
parts := strings.SplitN(header, "=", 2) | ||
headersMap[parts[0]] = parts[1] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add validation to prevent panic with malformed headers
The current implementation assumes that splitting the header always produces at least two parts, which could cause a panic if a malformed header is provided (e.g., a header without an '=' character).
Apply this diff to fix the issue:
headersMap := make(map[string]string)
for _, header := range otelMetricsExporterOTLPHeaders {
parts := strings.SplitN(header, "=", 2)
- headersMap[parts[0]] = parts[1]
+ if len(parts) == 2 {
+ headersMap[parts[0]] = parts[1]
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
otelMetricsExporterOTLPHeaders, _ := cmd.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag) | |
headersMap := make(map[string]string) | |
for _, header := range otelMetricsExporterOTLPHeaders { | |
parts := strings.SplitN(header, "=", 2) | |
headersMap[parts[0]] = parts[1] | |
} | |
otelMetricsExporterOTLPHeaders, _ := cmd.Flags().GetStringSlice(OtelMetricsExporterOTLPHeadersFlag) | |
headersMap := make(map[string]string) | |
for _, header := range otelMetricsExporterOTLPHeaders { | |
parts := strings.SplitN(header, "=", 2) | |
if len(parts) == 2 { | |
headersMap[parts[0]] = parts[1] | |
} | |
} |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #333 +/- ##
==========================================
- Coverage 25.62% 25.42% -0.21%
==========================================
Files 117 117
Lines 5062 5094 +32
==========================================
- Hits 1297 1295 -2
- Misses 3665 3699 +34
Partials 100 100 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds
OtelMetricsExporterOTLPHeadersFlag
to be able to add headers to theOTEL
Endpoint