fix: enable telemetry configuration #39
Conversation
Add configuration for supported values, including default documentation.
WalkthroughThe recent updates focus on enhancing observability within the project's infrastructure. Changes include added instructions and configurations for environment setup, integration of new observability-related variables in Docker, and updates in the Go codebase to support telemetry features like tracing and metrics, based on configurable settings. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
=======================================
Coverage 88.73% 88.73%
=======================================
Files 12 12
Lines 426 426
=======================================
Hits 378 378
Misses 33 33
Partials 15 15 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (1)
internal/observe/telemetry.go (1)
[!TIP]
Codebase VerificationThe search for configuration validation within the
newTraceProviderandnewMeterProviderfunctions did not reveal any specific validation logic directly within these functions. The output shows some conditional checks and error handling in a different function (Configure), which checks if certain features are enabled and handles errors, but this does not directly confirm that the configuration values used innewTraceProviderandnewMeterProviderare validated before use.Given this context, it appears that the configuration values might not be explicitly validated within the functions in question before they are used. This could potentially lead to runtime errors if invalid configuration values are passed to these functions.
Recommendation:
- It would be prudent to implement explicit validation for the configuration values within
newTraceProviderandnewMeterProvider, or ensure that any values passed to these functions are validated beforehand.- The development team should consider reviewing the configuration management strategy to ensure robustness and error handling.
Analysis chain
Line range hint
97-133: Verify that the configuration values used innewTraceProviderandnewMeterProviderare validated before use to prevent runtime errors.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the validation of configuration values before use in `newTraceProvider` and `newMeterProvider`. # Test: Search for validation logic for configuration values in the `telemetry.go` file. Expect: At least one occurrence of validation logic. rg --type go 'validateConfig' internal/observe/telemetry.goLength of output: 59
Script:
#!/bin/bash # Description: Search for any form of configuration validation in the `newTraceProvider` and `newMeterProvider` functions. # Search for any inline validation or error handling related to configuration values in the `newTraceProvider` function. echo "Searching in newTraceProvider:" rg --type go --context 5 'cfg\.' internal/observe/telemetry.go | grep -E 'if|error' # Search for any inline validation or error handling related to configuration values in the `newMeterProvider` function. echo "Searching in newMeterProvider:" rg --type go --context 5 'cfg\.' internal/observe/telemetry.go | grep -E 'if|error'Length of output: 965
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- .envrc (2 hunks)
- integration/docker-compose.yaml (1 hunks)
- internal/config/config.go (1 hunks)
- internal/observe/telemetry.go (4 hunks)
Files skipped from review due to trivial changes (1)
- .envrc
Additional comments not posted (2)
integration/docker-compose.yaml (1)
29-32: Ensure that the new environment variables are documented in the project's README or relevant documentation.internal/config/config.go (1)
45-50: Confirm that the default values and environment variable names for the new fields inObserveConfigalign with the project's configuration standards.
| tracerProvider, err := newTraceProvider(ctx, cfg, exporters) | ||
| if err != nil { | ||
| handleErr(err) | ||
| return | ||
| } | ||
| shutdownFuncs = append(shutdownFuncs, tracerProvider.Shutdown) | ||
| otel.SetTracerProvider(tracerProvider) | ||
|
|
||
| meterProvider, err := newMeterProvider(ctx, exporters) | ||
| if err != nil { | ||
| handleErr(err) | ||
| return | ||
| if cfg.MetricsEnabled { | ||
| meterProvider, err := newMeterProvider(ctx, cfg, exporters) | ||
| if err != nil { | ||
| handleErr(err) | ||
| return shutdown, err | ||
| } | ||
| shutdownFuncs = append(shutdownFuncs, meterProvider.Shutdown) | ||
| otel.SetMeterProvider(meterProvider) |
There was a problem hiding this comment.
Review the error handling in the instantiation of tracerProvider and meterProvider. Ensure that errors are properly logged and handled without causing unexpected behavior.
- handleErr(err)
+ if err != nil {
+ handleErr(err)
+ return
+ }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.
| tracerProvider, err := newTraceProvider(ctx, cfg, exporters) | |
| if err != nil { | |
| handleErr(err) | |
| return | |
| } | |
| shutdownFuncs = append(shutdownFuncs, tracerProvider.Shutdown) | |
| otel.SetTracerProvider(tracerProvider) | |
| meterProvider, err := newMeterProvider(ctx, exporters) | |
| if err != nil { | |
| handleErr(err) | |
| return | |
| if cfg.MetricsEnabled { | |
| meterProvider, err := newMeterProvider(ctx, cfg, exporters) | |
| if err != nil { | |
| handleErr(err) | |
| return shutdown, err | |
| } | |
| shutdownFuncs = append(shutdownFuncs, meterProvider.Shutdown) | |
| otel.SetMeterProvider(meterProvider) | |
| tracerProvider, err := newTraceProvider(ctx, cfg, exporters) | |
| if err != nil { | |
| if err != nil { | |
| handleErr(err) | |
| return | |
| } | |
| } | |
| shutdownFuncs = append(shutdownFuncs, tracerProvider.Shutdown) | |
| otel.SetTracerProvider(tracerProvider) | |
| if cfg.MetricsEnabled { | |
| meterProvider, err := newMeterProvider(ctx, cfg, exporters) | |
| if err != nil { | |
| if err != nil { | |
| handleErr(err) | |
| return shutdown, err | |
| } | |
| } | |
| shutdownFuncs = append(shutdownFuncs, meterProvider.Shutdown) | |
| otel.SetMeterProvider(meterProvider) |
Add configuration for supported values, including default documentation.
Some settings may have been configurable with OTel defaults, but for this small set it's useful to control directly.
See also
OTel default environment documentation: https://opentelemetry.io/docs/specs/otel/protocol/exporter/
Summary by CodeRabbit
Documentation
.envrc.New Features
docker-compose.yaml.Refactor