Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ conditionalTest({ min: 16 })('outgoing fetch', () => {
createTestServer(done)
.get('/api/v0', headers => {
expect(headers['baggage']).toEqual(expect.any(String));
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
})
.get('/api/v1', headers => {
expect(headers['baggage']).toEqual(expect.any(String));
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
})
.get('/api/v2', headers => {
Expand Down
6 changes: 4 additions & 2 deletions packages/cloudflare/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
SEMANTIC_ATTRIBUTE_URL_FULL,
captureException,
continueTrace,
flush,
Expand Down Expand Up @@ -45,8 +47,8 @@ export function wrapRequestHandler(
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.cloudflare',
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server',
['http.request.method']: request.method,
['url.full']: request.url,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I replaced these in a couple of places since I just added temporary constants for them

[SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD]: request.method,
[SEMANTIC_ATTRIBUTE_URL_FULL]: request.url,
};

const contentLength = request.headers.get('content-length');
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/semanticAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,7 @@ export const SEMANTIC_ATTRIBUTE_CACHE_HIT = 'cache.hit';
export const SEMANTIC_ATTRIBUTE_CACHE_KEY = 'cache.key';

export const SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE = 'cache.item_size';

/** TODO: Remove these once we update to latest semantic conventions */
export const SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD = 'http.request.method';
export const SEMANTIC_ATTRIBUTE_URL_FULL = 'url.full';
4 changes: 2 additions & 2 deletions packages/node/src/integrations/node-fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {
name: 'NodeFetch',
setupOnce() {
const instrumentation = new UndiciInstrumentation({
requireParentforSpans: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a typo in our config or upstream, seems like it should be requireParentForSpans 😅

Copy link
Collaborator Author

@timfish timfish Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignoreRequestHook: request => {
const url = getAbsoluteUrl(request.origin, request.path);
const tracingDisabled = !hasTracingEnabled();
const shouldIgnore = _ignoreOutgoingRequests && url && _ignoreOutgoingRequests(url);

if (shouldIgnore) {
Expand All @@ -50,7 +50,7 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => {

// If tracing is disabled, we still want to propagate traces
// So we do that manually here, matching what the instrumentation does otherwise
if (tracingDisabled) {
if (!hasTracingEnabled()) {
const ctx = context.active();
const addedHeaders: Record<string, string> = {};

Expand Down
3 changes: 2 additions & 1 deletion packages/opentelemetry/src/propagator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { propagation, trace } from '@opentelemetry/api';
import { W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core';
import { SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions';
import type { continueTrace } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_URL_FULL } from '@sentry/core';
import { hasTracingEnabled } from '@sentry/core';
import { getRootSpan } from '@sentry/core';
import { spanToJSON } from '@sentry/core';
Expand Down Expand Up @@ -293,7 +294,7 @@ function getExistingBaggage(carrier: unknown): string | undefined {
*/
function getCurrentURL(span: Span): string | undefined {
const spanData = spanToJSON(span).data;
const urlAttribute = spanData?.[SEMATTRS_HTTP_URL] || spanData?.['url.full'];
const urlAttribute = spanData?.[SEMATTRS_HTTP_URL] || spanData?.[SEMANTIC_ATTRIBUTE_URL_FULL];
if (urlAttribute) {
return urlAttribute;
}
Expand Down
8 changes: 5 additions & 3 deletions packages/opentelemetry/src/sampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@ import { TraceState } from '@opentelemetry/core';
import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base';
import { SamplingDecision } from '@opentelemetry/sdk-trace-base';
import {
SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
SEMANTIC_ATTRIBUTE_URL_FULL,
hasTracingEnabled,
sampleSpan,
} from '@sentry/core';
Expand Down Expand Up @@ -50,11 +52,11 @@ export class SentrySampler implements Sampler {
return wrapSamplingDecision({ decision: undefined, context, spanAttributes });
}

// If we have a http.client span that has no local parent, we never want to sample it
// If we have a http.client or undici span that has no local parent, we never want to sample it
// but we want to leave downstream sampling decisions up to the server
if (
spanKind === SpanKind.CLIENT &&
spanAttributes[SEMATTRS_HTTP_METHOD] &&
(spanAttributes[SEMATTRS_HTTP_METHOD] || spanAttributes[SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD]) &&
(!parentSpan || parentContext?.isRemote)
) {
return wrapSamplingDecision({ decision: undefined, context, spanAttributes });
Expand Down Expand Up @@ -196,7 +198,7 @@ function getBaseTraceState(context: Context, spanAttributes: SpanAttributes): Tr
let traceState = parentContext?.traceState || new TraceState();

// We always keep the URL on the trace state, so we can access it in the propagator
const url = spanAttributes[SEMATTRS_HTTP_URL] || spanAttributes['url.full'];
const url = spanAttributes[SEMATTRS_HTTP_URL] || spanAttributes[SEMANTIC_ATTRIBUTE_URL_FULL];
if (url && typeof url === 'string') {
traceState = traceState.set(SENTRY_TRACE_STATE_URL, url);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry/src/utils/isSentryRequest.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions';
import { getClient, isSentryRequestUrl } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_URL_FULL, getClient, isSentryRequestUrl } from '@sentry/core';

import type { AbstractSpan } from '../types';
import { spanHasAttributes } from './spanTypes';
Expand All @@ -16,7 +16,7 @@ export function isSentryRequestSpan(span: AbstractSpan): boolean {

const { attributes } = span;

const httpUrl = attributes[SEMATTRS_HTTP_URL] || attributes['url.full'];
const httpUrl = attributes[SEMATTRS_HTTP_URL] || attributes[SEMANTIC_ATTRIBUTE_URL_FULL];

if (!httpUrl) {
return false;
Expand Down
8 changes: 6 additions & 2 deletions packages/opentelemetry/src/utils/parseSpanDescription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import {
import type { SpanAttributes, TransactionSource } from '@sentry/types';
import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_URL_FULL,
} from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes';
import type { AbstractSpan } from '../types';
import { getSpanKind } from './getSpanKind';
Expand Down Expand Up @@ -213,7 +217,7 @@ export function getSanitizedUrl(
// This is the relative path of the URL, e.g. /sub
const httpTarget = attributes[SEMATTRS_HTTP_TARGET];
// This is the full URL, including host & query params etc., e.g. https://example.com/sub?foo=bar
const httpUrl = attributes[SEMATTRS_HTTP_URL] || attributes['url.full'];
const httpUrl = attributes[SEMATTRS_HTTP_URL] || attributes[SEMANTIC_ATTRIBUTE_URL_FULL];
// This is the normalized route name - may not always be available!
const httpRoute = attributes[SEMATTRS_HTTP_ROUTE];

Expand Down