Traceparent header: Added traceparent header to scrape request #16425
Conversation
This file contains hidden or 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
machine424
reviewed
Apr 24, 2025
|
Also a possible solution for #13647 (maybe to be added in a separate PR). |
will address this in different PR. |
machine424
reviewed
May 13, 2025
|
Hello! @machine424 I hope this approach works now, I have altered the targetScraper instance within |
machine424
requested changes
Jul 7, 2025
There was a problem hiding this comment.
I can still see the following issues:
- the
otelhttptrace.NewClientTracesetup is duplicated. - the test doesn't really test any logic from the source code, it does its own
otelhttptrace.NewClientTracesetup. withOTELGlobalsis a little big complicated.
here is my suggestion (providing the diff would make everyone life easier)
- put the setup in a
newScrapeClientthat we can really test. - use
t.Cleanupto simplify the util. - other miscs
diff --git a/scrape/scrape.go b/scrape/scrape.go
index 02be3c519..85a09407d 100644
--- a/scrape/scrape.go
+++ b/scrape/scrape.go
@@ -149,16 +149,10 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed
logger = promslog.NewNopLogger()
}
- client, err := config_util.NewClientFromConfig(cfg.HTTPClientConfig, cfg.JobName, options.HTTPClientOptions...)
+ client, err := newScrapeClient(cfg.HTTPClientConfig, cfg.JobName, options.HTTPClientOptions...)
if err != nil {
- return nil, fmt.Errorf("error creating HTTP client: %w", err)
+ return nil, err
}
- t := client.Transport
- client.Transport = otelhttp.NewTransport(
- t,
- otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
- return otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithoutSubSpans())
- }))
validationScheme, err := config.ToValidationScheme(cfg.MetricNameValidationScheme)
if err != nil {
return nil, fmt.Errorf("invalid metric name validation scheme: %w", err)
@@ -325,17 +319,11 @@ func (sp *scrapePool) reload(cfg *config.ScrapeConfig) error {
sp.metrics.targetScrapePoolReloads.Inc()
start := time.Now()
- client, err := config_util.NewClientFromConfig(cfg.HTTPClientConfig, cfg.JobName, sp.httpOpts...)
+ client, err := newScrapeClient(cfg.HTTPClientConfig, cfg.JobName, sp.httpOpts...)
if err != nil {
sp.metrics.targetScrapePoolReloadsFailed.Inc()
- return fmt.Errorf("error creating HTTP client: %w", err)
+ return err
}
- t := client.Transport
- client.Transport = otelhttp.NewTransport(
- t,
- otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
- return otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithoutSubSpans())
- }))
reuseCache := reusableCache(sp.config, cfg)
sp.config = cfg
@@ -2294,3 +2282,16 @@ func pickSchema(bucketFactor float64) int32 {
return int32(floor)
}
}
+
+func newScrapeClient(cfg config_util.HTTPClientConfig, name string, optFuncs ...config_util.HTTPClientOption) (*http.Client, error) {
+ client, err := config_util.NewClientFromConfig(cfg, name, optFuncs...)
+ if err != nil {
+ return nil, fmt.Errorf("error creating HTTP client: %w", err)
+ }
+ client.Transport = otelhttp.NewTransport(
+ client.Transport,
+ otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
+ return otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithoutSubSpans())
+ }))
+ return client, nil
+}
diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go
index c01931626..fce7f3003 100644
--- a/scrape/scrape_test.go
+++ b/scrape/scrape_test.go
@@ -24,7 +24,6 @@ import (
"math"
"net/http"
"net/http/httptest"
- "net/http/httptrace"
"net/url"
"os"
"sort"
@@ -38,14 +37,6 @@ import (
"github.com/gogo/protobuf/proto"
"github.com/google/go-cmp/cmp"
"github.com/grafana/regexp"
- "github.com/stretchr/testify/require"
- "go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace"
- "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
- "go.opentelemetry.io/otel"
- "go.opentelemetry.io/otel/propagation"
- sdktrace "go.opentelemetry.io/otel/sdk/trace"
- "go.opentelemetry.io/otel/trace"
-
"github.com/prometheus/client_golang/prometheus"
prom_testutil "github.com/prometheus/client_golang/prometheus/testutil"
dto "github.com/prometheus/client_model/go"
@@ -53,6 +44,11 @@ import (
"github.com/prometheus/common/expfmt"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
+ "github.com/stretchr/testify/require"
+ "go.opentelemetry.io/otel"
+ "go.opentelemetry.io/otel/propagation"
+ sdktrace "go.opentelemetry.io/otel/sdk/trace"
+
"github.com/prometheus/prometheus/config"
"github.com/prometheus/prometheus/discovery"
"github.com/prometheus/prometheus/discovery/targetgroup"
@@ -3119,63 +3115,55 @@ func TestAcceptHeader(t *testing.T) {
}
}
-// withOTELGlobals temporarily sets the global TracerProvider and Propagator
+// setupTracing temporarily sets the global TracerProvider and Propagator
// and restores the original state after the test completes.
-func withOTELGlobals(t *testing.T, tp trace.TracerProvider, propagator propagation.TextMapPropagator, testFunc func()) {
+func setupTracing(t *testing.T) {
t.Helper()
origTracerProvider := otel.GetTracerProvider()
origPropagator := otel.GetTextMapPropagator()
+ tp := sdktrace.NewTracerProvider(sdktrace.WithSampler(sdktrace.AlwaysSample()))
otel.SetTracerProvider(tp)
- otel.SetTextMapPropagator(propagator)
+ otel.SetTextMapPropagator(propagation.TraceContext{})
- defer func() {
+ t.Cleanup(func() {
otel.SetTracerProvider(origTracerProvider)
otel.SetTextMapPropagator(origPropagator)
- }()
-
- testFunc()
+ })
}
-// verifies that the HTTP client used by the target scraper propagates the OpenTelemetry "traceparent" header correctly.
-func TestRequestHeaderTraceparent(t *testing.T) {
- tp := sdktrace.NewTracerProvider(sdktrace.WithSampler(sdktrace.AlwaysSample()))
- withOTELGlobals(t, tp, propagation.TraceContext{}, func() {
- server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
- traceparent := r.Header.Get("traceparent")
- require.NotEmpty(t, traceparent)
- }))
- defer server.Close()
+// TestRequestTraceparentHeader verifies that the HTTP client used by the target scraper
+// propagates the OpenTelemetry "traceparent" header correctly.
+func TestRequestTraceparentHeader(t *testing.T) {
+ setupTracing(t)
- serverURL, err := url.Parse(server.URL)
- require.NoError(t, err)
+ server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+ // the traceparent header is sent.
+ require.NotEmpty(t, r.Header.Get("traceparent"))
+ }))
+ defer server.Close()
+ serverURL, err := url.Parse(server.URL)
+ require.NoError(t, err)
- client := &http.Client{
- Transport: otelhttp.NewTransport(
- http.DefaultTransport,
- otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
- return otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithoutSubSpans())
- }),
- ),
- }
+ client, err := newScrapeClient(config_util.DefaultHTTPClientConfig, "test")
+ require.NoError(t, err)
- ts := &targetScraper{
- Target: &Target{
- labels: labels.FromStrings(
- model.SchemeLabel, serverURL.Scheme,
- model.AddressLabel, serverURL.Host,
- ),
- scrapeConfig: &config.ScrapeConfig{},
- },
- client: client,
- }
+ ts := &targetScraper{
+ Target: &Target{
+ labels: labels.FromStrings(
+ model.SchemeLabel, serverURL.Scheme,
+ model.AddressLabel, serverURL.Host,
+ ),
+ scrapeConfig: &config.ScrapeConfig{},
+ },
+ client: client,
+ }
- resp, err := ts.scrape(context.Background())
- require.NoError(t, err)
- require.NotNil(t, resp)
- defer resp.Body.Close()
- })
+ resp, err := ts.scrape(context.Background())
+ require.NoError(t, err)
+ require.NotNil(t, resp)
+ defer resp.Body.Close()
}
func TestTargetScraperScrapeOK(t *testing.T) {73fda55 to
f83c51e
Compare
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com> (cherry picked from commit 44a620d) Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com> (cherry picked from commit 97f288a) Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com> (cherry picked from commit d5dd861) Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
Signed-off-by: Vanshikav123 <vanshikav928@gmail.com> (cherry picked from commit 3cd8092) Signed-off-by: Vanshikav123 <vanshikav928@gmail.com>
|
could you elaborate on why the was removed? |
while analyzing the code flow, i thought the core of the instrumentation is applied when the http.Client is first created for the scrapePool in the newScrapeClient function, the sync() method does not create a new client. Instead, it injects pre-instrumented client (sp.client) into each targetScraper instance? 🤔 |
|
We want to add the parent/wrapping prometheus/storage/remote/client.go Line 279 in 8eb445b To have more context about what the automatically created spans. We would get a GET /metrics span, and we wouldn't know what for.
|
oh okay ! review addressed , thank you. |
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.
Initial Fix for #14321