feat: upgrade otel dependencies with backward compatible semconv attributes#2714
Open
feat: upgrade otel dependencies with backward compatible semconv attributes#2714
Conversation
Noroth
requested review from
a team,
SkArchon,
StarpTech,
devsergiy,
endigma,
jensneuse and
pepol
as code owners
…cies-and-implement-semconv
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated Go toolchain and many module versions; introduced attribute-filtering tracer provider and semconv remapping; dropped otelhttp-scope metrics; set HTTP content-length attributes on client spans; and adjusted telemetry tests and metric cardinality handling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency ReviewThe following issues were found:
|
Router-nonroot image scan passed✅ No security vulnerabilities found in image: |
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (47.57%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2714 +/- ##
===========================================
+ Coverage 41.55% 64.68% +23.13%
===========================================
Files 785 270 -515
Lines 112455 27424 -85031
Branches 8667 0 -8667
===========================================
- Hits 46730 17740 -28990
+ Misses 65362 8262 -57100
- Partials 363 1422 +1059
🚀 New features to boost your workflow:
|
…cies-and-implement-semconv
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
router-tests/telemetry/telemetry_helpers_test.go (2)
14-19: Consider simplifying:HasValuecheck is redundant.The
attributes.Value(key)call on line 16-17 already returnsok = falsewhen the key doesn't exist, making theHasValuecheck on line 15 redundant.♻️ Suggested simplification
func assertHasAttributes(t *testing.T, attributes attribute.Set, expectedAttributes ...attribute.KeyValue) { t.Helper() for _, expectedAttribute := range expectedAttributes { - assert.True(t, attributes.HasValue(expectedAttribute.Key)) value, ok := attributes.Value(expectedAttribute.Key) - assert.True(t, ok) + assert.True(t, ok, "expected attribute %s to exist", expectedAttribute.Key) assert.Equal(t, expectedAttribute.Value, value) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_helpers_test.go` around lines 14 - 19, The HasValue check is redundant because attributes.Value(expectedAttribute.Key) already returns ok=false for missing keys; remove the call to attributes.HasValue and its assertion, and instead call value, ok := attributes.Value(expectedAttribute.Key) directly, assert.True(t, ok) to ensure the key exists, then assert.Equal(t, expectedAttribute.Value, value); update the loop in the test (referring to attributes, expectedAttribute, HasValue, and Value) accordingly to simplify the assertions.
22-26: Debug helper:AsString()returns empty string for non-string attribute types.This is fine for debugging purposes, but be aware that
AsString()returns an empty string for non-string attribute types (Int, Float, Bool, etc.). If you need to debug non-string attributes, consider usingattribute.Value.Emit()or type-specific getters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_helpers_test.go` around lines 22 - 26, The debug helper printAttributeNames uses attribute.Value.AsString(), which yields an empty string for non-string types; update printAttributeNames to handle non-string attributes by using attribute.Value.Emit() or the type-specific getters (e.g., Int, Bool, Double) or a type switch on attribute.Value.Type to produce meaningful output for numeric/boolean attributes so debugging shows actual values instead of blanks.router/go.mod (1)
35-36: Stale comment contradicts the actual change.The comment says "Do not upgrade, it renames attributes we rely on" but the line immediately below shows
otelhttp v0.67.0which is the upgraded version. This comment appears to be leftover from before this PR and is now misleading.Consider removing or updating this comment to reflect the new strategy (using
FilteringTracerProviderto maintain backward compatibility):- // Do not upgrade, it renames attributes we rely on + // Upgraded with backward-compatible attribute filtering via FilteringTracerProvider go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/go.mod` around lines 35 - 36, The inline comment stating "Do not upgrade, it renames attributes we rely on" is stale and contradicts the actual module line "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0"; update or remove that comment so it reflects the new approach using FilteringTracerProvider (or simply delete the warning) next to the "otelhttp v0.67.0" entry to avoid confusion for future readers.router-tests/telemetry/telemetry_test.go (3)
11146-11148: Don’t assert absence on onlyDataPoints[0].These checks only inspect the first histogram datapoint. If multiple datapoints exist, the test can miss a violating datapoint carrying the forbidden key.
💡 Suggested fix pattern
- atts := subgraphNonMetric.Data.(metricdata.Histogram[float64]).DataPoints[0].Attributes - _, ok := atts.Value(attribute.Key(key)) - require.False(t, ok) + for _, dp := range subgraphNonMetric.Data.(metricdata.Histogram[float64]).DataPoints { + _, ok := dp.Attributes.Value(attribute.Key(key)) + require.False(t, ok) + }Also applies to: 11214-11216, 11296-11298
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` around lines 11146 - 11148, The test currently only inspects the first histogram datapoint (subgraphNonMetric.Data.(metricdata.Histogram[float64]).DataPoints[0]) when asserting the forbidden attribute is absent; update the check to iterate over all datapoints in the DataPoints slice and assert that for each datapoint's Attributes the attribute.Key(key) is not present (use the same attribute lookup pattern and require.False/assertion per datapoint) so no datapoint can carry the forbidden key; apply the same change where similar single-index checks appear (the other instances noted).
4435-4441: Clean up commented-out assertions.These commented
require.Containslines are obsolete afterassertHasAttributesand should be removed to keep the test readable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` around lines 4435 - 4441, Remove the obsolete commented assertions (the lines starting with "// require.Contains(t, sn[0].Attributes(), ...") since they duplicate checks now covered by assertHasAttributes; locate them near the telemetry test where assertHasAttributes is used and delete those commented require.Contains lines so the test file is cleaner and only relies on assertHasAttributes for the attribute checks.
7965-7965: Remove leftover debug output from test path.Line 7965 prints attribute names unconditionally, which adds noise to CI logs and makes failures harder to triage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` at line 7965, Remove the leftover debug print by deleting the call to printAttributeNames(sn[2].Attributes()) in the test; if you need the info for debugging keep it behind the test logger (e.g., use t.Log/T.Logf) or a conditional debug flag, but do not leave an unconditional printAttributeNames invocation that writes to stdout in the test path.router/pkg/trace/filtering_provider.go (1)
35-37: Prefer OTEL'sembeddedinterfaces in these wrappers.
FilteringTracerProvider,filteringTracer, andfilteringSpanembedoteltrace.TracerProvider,oteltrace.Tracer, andoteltrace.Spandirectly. The OTEL trace docs explicitly warn that these interfaces may gain methods in minor releases and recommend embeddinggo.opentelemetry.io/otel/trace/embedded(ortrace/noop) instead, so upgrades fail predictably instead of turning into runtime panics. (pkg.go.dev)Also applies to: 43-45, 84-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/pkg/trace/filtering_provider.go` around lines 35 - 37, Replace direct embedding of OTEL interfaces with OTEL's embedded variants to avoid breakage when methods are added: change the structs FilteringTracerProvider, filteringTracer, and filteringSpan to embed go.opentelemetry.io/otel/trace/embedded versions (e.g., embedded.TracerProvider, embedded.Tracer, embedded.Span or the trace/noop equivalents) instead of oteltrace.TracerProvider, oteltrace.Tracer, and oteltrace.Span; update imports to reference the embedded package and ensure method forwarding remains correct so the wrappers still satisfy the same interfaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@router-plugin/go.mod`:
- Line 17: Update the grpc dependency entry for google.golang.org/grpc in go.mod
from v1.79.2 to v1.79.3 to pull in the security fix for GHSA-p77j-4mvh-x3m3;
after changing the version string, run dependency resolution (e.g., go get
google.golang.org/grpc@v1.79.3 and go mod tidy) and verify builds/tests that
reference the gRPC client/server symbols to ensure no regressions.
In `@router-tests/telemetry/telemetry_test.go`:
- Line 5524: The test is indexing rm.ScopeMetrics by position after already
resolving the scope by name, which reintroduces order coupling; update the
assertion to use the previously resolved scopeMetric (e.g.,
scopeMetric.Metrics[6]) or locate the specific metric within scopeMetric.Metrics
by name before calling metricdatatest.AssertEqual, keeping the assertion target
consistent with the earlier lookup (use scopeMetric instead of
rm.ScopeMetrics[0] when asserting routerInfoMetric with
metricdatatest.AssertEqual).
In `@router/pkg/trace/filtering_provider.go`:
- Around line 88-96: filteringSpan.SetAttributes currently compacts the variadic
kv slice in place which mutates caller-owned backing storage; instead, allocate
a new slice (e.g., make([]attribute.KeyValue, 0, len(kv))) or preallocate
filtered of length n and append acceptable entries from kv while checking
semconvDropKeys, and then call s.Span.SetAttributes(filtered...) so the original
kv is not modified; update the function to use this new filtered slice and
remove in-place writes to kv.
- Around line 67-77: The current rebuild loop in filtering_provider.go
incorrectly drops all SpanStartEventOption implementations (notably
WithTimestamp) when filtering attributes; update the rebuild logic in the span
start path (the code that mutates opts and uses cfg) to preserve explicit start
options by extracting the preserved values from cfg (timestamp via
ContextWithStartTime/WithTimestamp, span kind, new root flag, links) and then
reconstruct opts: copy all non-attribute SpanStartOption values, re-add
oteltrace.WithAttributes(filtered...), and explicitly re-add the preserved
options (e.g., oteltrace.WithTimestamp(timestamp) if set,
oteltrace.WithSpanKind(kind) if set, oteltrace.WithNewRoot(true) if set, and any
links) so WithTimestamp and other important start options are not lost.
In `@router/pkg/trace/transport.go`:
- Around line 49-52: The code calls span.SetAttributes with
semconv.HTTPRequestContentLength(int(r.ContentLength)) (and similarly for
res.ContentLength) which can truncate int64 to int on 32-bit systems or for very
large payloads; update the handling in the functions that use r.ContentLength
and res.ContentLength (e.g., where otrace.SpanFromContext is used and
span.SetAttributes is invoked) to defensively check the int64 value before
casting: if the value fits within int (use math.MaxInt/int64 comparisons) cast
normally, otherwise clamp to math.MaxInt or use a semantic attribute helper that
accepts int64 if available, and include a short comment explaining the clamp to
avoid silent truncation.
---
Nitpick comments:
In `@router-tests/telemetry/telemetry_helpers_test.go`:
- Around line 14-19: The HasValue check is redundant because
attributes.Value(expectedAttribute.Key) already returns ok=false for missing
keys; remove the call to attributes.HasValue and its assertion, and instead call
value, ok := attributes.Value(expectedAttribute.Key) directly, assert.True(t,
ok) to ensure the key exists, then assert.Equal(t, expectedAttribute.Value,
value); update the loop in the test (referring to attributes, expectedAttribute,
HasValue, and Value) accordingly to simplify the assertions.
- Around line 22-26: The debug helper printAttributeNames uses
attribute.Value.AsString(), which yields an empty string for non-string types;
update printAttributeNames to handle non-string attributes by using
attribute.Value.Emit() or the type-specific getters (e.g., Int, Bool, Double) or
a type switch on attribute.Value.Type to produce meaningful output for
numeric/boolean attributes so debugging shows actual values instead of blanks.
In `@router-tests/telemetry/telemetry_test.go`:
- Around line 11146-11148: The test currently only inspects the first histogram
datapoint (subgraphNonMetric.Data.(metricdata.Histogram[float64]).DataPoints[0])
when asserting the forbidden attribute is absent; update the check to iterate
over all datapoints in the DataPoints slice and assert that for each datapoint's
Attributes the attribute.Key(key) is not present (use the same attribute lookup
pattern and require.False/assertion per datapoint) so no datapoint can carry the
forbidden key; apply the same change where similar single-index checks appear
(the other instances noted).
- Around line 4435-4441: Remove the obsolete commented assertions (the lines
starting with "// require.Contains(t, sn[0].Attributes(), ...") since they
duplicate checks now covered by assertHasAttributes; locate them near the
telemetry test where assertHasAttributes is used and delete those commented
require.Contains lines so the test file is cleaner and only relies on
assertHasAttributes for the attribute checks.
- Line 7965: Remove the leftover debug print by deleting the call to
printAttributeNames(sn[2].Attributes()) in the test; if you need the info for
debugging keep it behind the test logger (e.g., use t.Log/T.Logf) or a
conditional debug flag, but do not leave an unconditional printAttributeNames
invocation that writes to stdout in the test path.
In `@router/go.mod`:
- Around line 35-36: The inline comment stating "Do not upgrade, it renames
attributes we rely on" is stale and contradicts the actual module line
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0"; update
or remove that comment so it reflects the new approach using
FilteringTracerProvider (or simply delete the warning) next to the "otelhttp
v0.67.0" entry to avoid confusion for future readers.
In `@router/pkg/trace/filtering_provider.go`:
- Around line 35-37: Replace direct embedding of OTEL interfaces with OTEL's
embedded variants to avoid breakage when methods are added: change the structs
FilteringTracerProvider, filteringTracer, and filteringSpan to embed
go.opentelemetry.io/otel/trace/embedded versions (e.g., embedded.TracerProvider,
embedded.Tracer, embedded.Span or the trace/noop equivalents) instead of
oteltrace.TracerProvider, oteltrace.Tracer, and oteltrace.Span; update imports
to reference the embedded package and ensure method forwarding remains correct
so the wrappers still satisfy the same interfaces.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b49a4b2d-9428-4416-a9d5-d32d3f070e59
⛔ Files ignored due to path filters (5)
demo/go.sumis excluded by!**/*.sumgraphqlmetrics/go.sumis excluded by!**/*.sumrouter-plugin/go.sumis excluded by!**/*.sumrouter-tests/go.sumis excluded by!**/*.sumrouter/go.sumis excluded by!**/*.sum
📒 Files selected for processing (14)
demo/go.modgraphqlmetrics/go.modrouter-plugin/go.modrouter-tests/go.modrouter-tests/telemetry/telemetry_helpers_test.gorouter-tests/telemetry/telemetry_test.gorouter/core/transport.gorouter/go.modrouter/pkg/metric/meter.gorouter/pkg/trace/filtering_provider.gorouter/pkg/trace/handler.gorouter/pkg/trace/meter.gorouter/pkg/trace/semconv_processor.gorouter/pkg/trace/transport.go
Comment on lines
+49
to
+52
| if r.ContentLength > 0 { | ||
| span := otrace.SpanFromContext(r.Context()) | ||
| span.SetAttributes(semconv.HTTPRequestContentLength(int(r.ContentLength))) | ||
| } |
Contributor
There was a problem hiding this comment.
Potential int64-to-int truncation for very large content lengths.
r.ContentLength and res.ContentLength are int64, but HTTPRequestContentLength and HTTPResponseContentLength accept int. On 32-bit systems (or when content exceeds math.MaxInt), this could truncate. While rare in practice, consider checking bounds or using a larger type if supported.
🛡️ Optional defensive check
+import "math"
+
// otelhttp v0.67.0 no longer emits http.request_content_length on client spans.
// Set it here for backward compatibility with downstream systems.
-if r.ContentLength > 0 {
+if r.ContentLength > 0 && r.ContentLength <= int64(math.MaxInt) {
span := otrace.SpanFromContext(r.Context())
span.SetAttributes(semconv.HTTPRequestContentLength(int(r.ContentLength)))
}Also applies to: 64-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@router/pkg/trace/transport.go` around lines 49 - 52, The code calls
span.SetAttributes with semconv.HTTPRequestContentLength(int(r.ContentLength))
(and similarly for res.ContentLength) which can truncate int64 to int on 32-bit
systems or for very large payloads; update the handling in the functions that
use r.ContentLength and res.ContentLength (e.g., where otrace.SpanFromContext is
used and span.SetAttributes is invoked) to defensively check the int64 value
before casting: if the value fits within int (use math.MaxInt/int64 comparisons)
cast normally, otherwise clamp to math.MaxInt or use a semantic attribute helper
that accepts int64 if available, and include a short comment explaining the
clamp to avoid silent truncation.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
router-tests/telemetry/telemetry_test.go (1)
5524-5524:⚠️ Potential issue | 🟠 MajorUse the resolved scope lookup here instead of indexing
rm.ScopeMetrics.This reintroduces scope-order coupling immediately after
scopeMetricwas looked up by name, so the test can fail because export order changed rather than becauserouter.inforegressed.💡 Suggested fix
- metricdatatest.AssertEqual(t, routerInfoMetric, rm.ScopeMetrics[0].Metrics[6], metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars()) + metricdatatest.AssertEqual(t, routerInfoMetric, *testutils.GetMetricByName(&scopeMetric, "router.info"), metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreExemplars())As per coding guidelines, "Sort collections by a stable key before making positional assertions on non-deterministic order items (metrics, spans, etc.)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` at line 5524, The test currently asserts against rm.ScopeMetrics[0].Metrics[6] which reintroduces ordering coupling; instead use the previously resolved scope lookup (the scopeMetric or scopeMetrics variable obtained by name) and assert against its Metrics slice (e.g., scopeMetric.Metrics[...] or the metrics slice returned by the lookup) when comparing routerInfoMetric with metricdatatest.AssertEqual; locate where rm.ScopeMetrics is indexed and replace that positional access with the resolved scope's Metrics to avoid relying on export order.
🧹 Nitpick comments (3)
router-tests/telemetry/telemetry_test.go (2)
7965-7965: Drop the leftover debug print before merging.This writes to test output on every run and makes already-parallel telemetry logs harder to read.
✂️ Cleanup
- printAttributeNames(sn[2].Attributes())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` at line 7965, Remove the leftover debug print call printAttributeNames(sn[2].Attributes()) from the telemetry_test.go test; locate the invocation (likely inside the test that iterates over span nodes or variable sn) and delete that line so the test no longer writes to stdout during runs and parallel telemetry logs remain clean.
4435-4441: Remove the commented-out assertion block.
assertHasAttributesalready replaced these checks, so keeping the oldrequire.Containscalls commented out just adds noise to an already dense test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router-tests/telemetry/telemetry_test.go` around lines 4435 - 4441, Remove the dead commented-out assertions block in telemetry_test.go (the lines with require.Contains referencing sn[0].Attributes() and otel.Wg* constants) because assertHasAttributes already covers those checks; delete the commented lines so only the active assertHasAttributes usage remains and the test is no longer cluttered by legacy commented assertions.router/pkg/trace/filtering_provider.go (1)
67-85: Consider preservingStackTracefor completeness.The rebuild logic preserves timestamp, span kind, new root flag, links, and attributes. However,
SpanConfigalso has aStackTrace()accessor forWithStackTrace(true). If any caller uses that option, it would be silently dropped.Since
otelhttpdoesn't typically set stack trace at span start, this is unlikely to cause issues in practice. Flagging for completeness in case future callers use this option.Optional fix to preserve StackTrace
if links := cfg.Links(); len(links) > 0 { rebuilt = append(rebuilt, oteltrace.WithLinks(links...)) } + if cfg.StackTrace() { + rebuilt = append(rebuilt, oteltrace.WithStackTrace(true)) + } if len(filtered) > 0 { rebuilt = append(rebuilt, oteltrace.WithAttributes(filtered...)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@router/pkg/trace/filtering_provider.go` around lines 67 - 85, The rebuild logic drops the SpanConfig.StackTrace setting so WithStackTrace(true) would be lost; update the reconstruction in filtering_provider.go to check cfg.StackTrace() and if true append oteltrace.WithStackTrace() to rebuilt (before assigning opts = rebuilt), ensuring the StackTrace flag from SpanConfig is preserved alongside Timestamp, SpanKind, NewRoot, Links, and Attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@router-tests/telemetry/telemetry_test.go`:
- Line 5524: The test currently asserts against rm.ScopeMetrics[0].Metrics[6]
which reintroduces ordering coupling; instead use the previously resolved scope
lookup (the scopeMetric or scopeMetrics variable obtained by name) and assert
against its Metrics slice (e.g., scopeMetric.Metrics[...] or the metrics slice
returned by the lookup) when comparing routerInfoMetric with
metricdatatest.AssertEqual; locate where rm.ScopeMetrics is indexed and replace
that positional access with the resolved scope's Metrics to avoid relying on
export order.
---
Nitpick comments:
In `@router-tests/telemetry/telemetry_test.go`:
- Line 7965: Remove the leftover debug print call
printAttributeNames(sn[2].Attributes()) from the telemetry_test.go test; locate
the invocation (likely inside the test that iterates over span nodes or variable
sn) and delete that line so the test no longer writes to stdout during runs and
parallel telemetry logs remain clean.
- Around line 4435-4441: Remove the dead commented-out assertions block in
telemetry_test.go (the lines with require.Contains referencing
sn[0].Attributes() and otel.Wg* constants) because assertHasAttributes already
covers those checks; delete the commented lines so only the active
assertHasAttributes usage remains and the test is no longer cluttered by legacy
commented assertions.
In `@router/pkg/trace/filtering_provider.go`:
- Around line 67-85: The rebuild logic drops the SpanConfig.StackTrace setting
so WithStackTrace(true) would be lost; update the reconstruction in
filtering_provider.go to check cfg.StackTrace() and if true append
oteltrace.WithStackTrace() to rebuilt (before assigning opts = rebuilt),
ensuring the StackTrace flag from SpanConfig is preserved alongside Timestamp,
SpanKind, NewRoot, Links, and Attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33a6d89d-4fb8-491f-b898-d81c80635d85
📒 Files selected for processing (2)
router-tests/telemetry/telemetry_test.gorouter/pkg/trace/filtering_provider.go
SkArchon
reviewed
Mar 31, 2026
SkArchon
reviewed
Mar 31, 2026
…cies-and-implement-semconv
alepane21
requested changes
Apr 1, 2026
| otelhttp.WithMeterProvider(sdkmetric.NewMeterProvider()), | ||
| otelhttp.WithSpanNameFormatter(s.SpanNameFormatter), | ||
| otelhttp.WithTracerProvider(s.TracerProvider), | ||
| otelhttp.WithTracerProvider(&FilteringTracerProvider{TracerProvider: s.TracerProvider}), |
Contributor
There was a problem hiding this comment.
Can we have this to be configurable per exporter? When exporting toward external services (like newrelic or datadog) I think it would be useful to allow the standard names, to make it easier to integrate with external services.
Contributor
Author
|
The current retry logic has some regressions because the request body is not rewinded automatically. As a workaround on each retry we have to rewind the body manually before calling the underlying transport. I opened an issue in the otel repository and the maintainers are taking a look currently |
…268-router-upgrade-otel-dependencies-and-implement-semconv
…cies-and-implement-semconv
thisisnithin
added a commit
that referenced
this pull request
Apr 13, 2026
Being fixed in #2714 which upgrades opentelemetry-go dependencies.
…cies-and-implement-semconv
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR is the first step of our OTEL upgrade. It ensures that the dependencies are upgraded to the latest version while we are still able to drop/rewrite any newly introduced/altered attributes. This is necessary because various telemetry systems currently rely on the attributes emitted by the router.
In the future we will need to slowly migrate to the new attributes
Summary by CodeRabbit
Chores
Improvements
Tests
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.