Add traceparent header to enable distributed tracing. #914
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hectorcast-db
approved these changes
May 7, 2024
mgyucht
commented
May 8, 2024
| @@ -99,7 +99,7 @@ type Config struct { | |||
| // Use at your own risk or for unit testing purposes. | |||
| InsecureSkipVerify bool `name:"skip_verify" auth:"-"` | |||
|
|
|||
| // Number of seconds for HTTP timeout. Default is 300 (5 minutes). | |||
| // Number of seconds for HTTP timeout. Default is 60 (1 minute). | |||
There was a problem hiding this comment.
This was a typo from before, the default has always been 60 seconds.
mgyucht
added a commit
that referenced
this pull request
May 14, 2024
* Fixed codecov for repository ([#909](#909)). * Add traceparent header to enable distributed tracing. ([#914](#914)). * Log cancelled and failed requests ([#919](#919)). Dependency updates: * Bump golang.org/x/net from 0.22.0 to 0.24.0 ([#884](#884)). * Bump golang.org/x/net from 0.17.0 to 0.23.0 in /examples/zerolog ([#896](#896)). * Bump golang.org/x/net from 0.21.0 to 0.23.0 in /examples/slog ([#897](#897)).
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 14, 2024
* Fixed codecov for repository ([#909](#909)). * Add traceparent header to enable distributed tracing. ([#914](#914)). * Log cancelled and failed requests ([#919](#919)). Dependency updates: * Bump golang.org/x/net from 0.22.0 to 0.24.0 ([#884](#884)). * Bump golang.org/x/net from 0.17.0 to 0.23.0 in /examples/zerolog ([#896](#896)). * Bump golang.org/x/net from 0.21.0 to 0.23.0 in /examples/slog ([#897](#897)).
github-merge-queue bot
pushed a commit
to databricks/cli
that referenced
this pull request
Nov 1, 2024
## Changes This PR adds the `cmd-exec-id` field to the user agent. This allows us to correlate multiple HTTP requests made from the CLI. ### Why Not Use HTTP traceparent? We considered using the traceparent header in HTTP as an alternative, but it's not a good fit for our use case. Here's why: 1. Purpose of traceparent: It's designed to trace a single HTTP request across a distributed system as it moves through subsystems and proxies. 2. Our requirement: We need to trace multiple HTTP requests made during a single command execution in the CLI. For more details about how traceparent itself works and how it's used in the Go SDK, see databricks/databricks-sdk-go#914. ## Tests Unit test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Currently, it is difficult to correlate requests made by a client with a request made by a backend service, especially if the same request is made multiple times. The REST API supports the Trace Context standard which defines a standard way of propagating tracing information through HTTP headers.
This PR implements a traceparent generator to construct new traceparent headers in accordance with this standard. These traceparents are attached to the headers of each individual request. The resulting header is visible when debug logs are enabled and DATABRICKS_DEBUG_HEADERS is set.
Tests
Added a unit test to ensure that new traceparents are set for every request.
It's hard to test that a debug log actually contains this header without rewriting a lot of the matching logic for debug logs, but I did need to remove traceparent from those unit tests, suggesting that debug logging does include traceparent (like any other header).
Manually tested that the traceparent set by the client is visible to the server by checking our internal access logs.
make testpassingmake fmtapplied