feat: Add support for propagateTraceparent#1277
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| { tracing, breadcrumbs }: NetOptions, | ||
| { enableLogs, tracePropagationTargets, propagateTraceparent }: ClientOptions, | ||
| ): WrappedRequestMethodFactory { |
There was a problem hiding this comment.
Bug: The getTraceData() function is called with an unsupported propagateTraceparent parameter. This option should be configured during SDK initialization, not passed directly to this function.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The getTraceData() function is being called with an object containing span and an unsupported propagateTraceparent parameter. The function's signature only accepts a Span or Scope object. Because propagateTraceparent is not a valid parameter for this function, it will be ignored, causing the feature to silently fail. Consequently, outgoing HTTP requests made through the Electron net module will not include the W3C traceparent header, even when the SDK is configured to do so, which defeats the purpose of the change.
💡 Suggested Fix
Remove the propagateTraceparent property from the object passed to getTraceData(). The SDK's instrumentation layer will automatically handle the traceparent header based on the global configuration provided during init(). The call should only pass the span object.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/main/integrations/net-breadcrumbs.ts#L90-L92
Potential issue: The `getTraceData()` function is being called with an object containing
`span` and an unsupported `propagateTraceparent` parameter. The function's signature
only accepts a `Span` or `Scope` object. Because `propagateTraceparent` is not a valid
parameter for this function, it will be ignored, causing the feature to silently fail.
Consequently, outgoing HTTP requests made through the Electron `net` module will not
include the W3C `traceparent` header, even when the SDK is configured to do so, which
defeats the purpose of the change.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7641719
There was a problem hiding this comment.
You couldn't be more wrong. It's in the type definitions.
| }, | ||
| spans: expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| op: 'http.client', |
There was a problem hiding this comment.
l: any way we can test against the traceparent header being part of the request?
No strong feelings, feel free to disregard since we test this in the JS SDKs.
2 participants
Closes #1275