fix: enable HTTP client traces in Open Telemetry output #41
Conversation
This allows more testing of concurrent operations.
WalkthroughThe recent updates focus on enhancing HTTP request tracing and improving the Buildkite integration setup. Changes include the introduction of environment variables for detailed HTTP tracing, expansion of Buildkite agents in Docker configurations, and refinements in error handling and telemetry within the codebase. These enhancements aim to increase observability and streamline CI/CD processes. 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 (
|
jamestelfer
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #41 +/- ##
==========================================
+ Coverage 88.73% 89.73% +0.99%
==========================================
Files 12 12
Lines 426 448 +22
==========================================
+ Hits 378 402 +24
+ Misses 33 32 -1
+ Partials 15 14 -1 ☔ 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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum
Files selected for processing (9)
- .envrc (1 hunks)
- integration/docker-compose.yaml (1 hunks)
- internal/buildkite/pipeline.go (2 hunks)
- internal/buildkite/pipeline_test.go (4 hunks)
- internal/config/config.go (1 hunks)
- internal/observe/mux.go (1 hunks)
- internal/observe/mux_test.go (2 hunks)
- internal/observe/telemetry.go (2 hunks)
- main.go (2 hunks)
Files skipped from review due to trivial changes (1)
- integration/docker-compose.yaml
Additional Context Used
GitHub Check Runs (1)
codecov/patch failure (2)
internal/buildkite/pipeline.go: [warning] 21-22: internal/buildkite/pipeline.go#L21-L22
Added lines #L21 - L22 were not covered by tests
internal/buildkite/pipeline.go: [warning] 29-30: internal/buildkite/pipeline.go#L29-L30
Added lines #L29 - L30 were not covered by tests
Additional comments not posted (12)
internal/observe/mux.go (2)
25-31: LGTM! The OpenTelemetry handler wrapping with route tagging is correctly implemented.
36-36: LGTM! Proper delegation of HTTP handling to the wrapped multiplexer.internal/observe/mux_test.go (1)
Line range hint
29-38: LGTM! The test correctly verifies the application of route tagging in theHandlemethod.internal/config/config.go (1)
45-52: LGTM! The new configuration fields for HTTP transport and connection tracing are correctly added with appropriate default values and environment bindings.internal/buildkite/pipeline.go (2)
15-16: LGTM! The changes to thePipelineLookupstruct support better modularity by decentralizing client creation.
54-80: LGTM! The custom round tripper and token authentication setup in thecreateClientmethod are correctly implemented to ensure context and authentication are included in API requests..envrc (1)
61-72: LGTM! The new environment variables for controlling HTTP transport and connection tracing are well-documented and align with the observability enhancements.internal/buildkite/pipeline_test.go (2)
41-45: LGTM! The updated tests for theNewfunction correctly include error handling checks, ensuring the robustness of client initialization.Also applies to: 82-86, 113-117, 135-139
45-45: LGTM! The tests for theRepositoryLookupmethod are comprehensive and cover crucial scenarios including error handling and token verification.Also applies to: 86-86, 117-117, 139-139
main.go (2)
42-46: LGTM! The error handling in the configuration of Buildkite and GitHub clients enhances the robustness of the server setup.
84-97: LGTM! The configuration of telemetry and the HTTP client in the server launch process are correctly implemented, ensuring proper error handling and data capture.internal/observe/telemetry.go (1)
82-97: LGTM! The conditional configuration of the HTTP transport with OpenTelemetry instrumentation is well-implemented, providing flexibility in tracing setup.
The context was not being used in calls, leading to disconnected traces. Using the context means creating the client each time, as the client does not support per-request context.
jamestelfer
force-pushed
the
jamestelfer/issue38
branch
from
ea7f920 to
826041f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- internal/buildkite/pipeline.go (2 hunks)
- internal/buildkite/pipeline_test.go (5 hunks)
- main.go (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- internal/buildkite/pipeline.go
- internal/buildkite/pipeline_test.go
- main.go
Enables HTTP client tracing in the Open Telemetry output.
The use of this functionality can be disabled if cost reduction is required.
Fixes #38
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation