Skip to content

Commit

Permalink
Merge branch 'develop' into fix/tracing-improve-network-protocol
Browse files Browse the repository at this point in the history
  • Loading branch information
k-fish authored Jul 17, 2023
2 parents 3b27b6f + 2919511 commit 377d74f
Show file tree
Hide file tree
Showing 13 changed files with 362 additions and 34 deletions.
2 changes: 1 addition & 1 deletion packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"build:types:core": "tsc -p tsconfig.types.json",
"build:types:downlevel": "yarn downlevel-dts build/npm/types build/npm/types-ts3.8 --to ts3.8",
"build:watch": "run-p build:transpile:watch build:bundle:watch build:types:watch",
"build:dev:watch": "yarn build:watch",
"build:dev:watch": "run-p build:transpile:watch build:types:watch",
"build:bundle:watch": "rollup -c rollup.bundle.config.js --watch",
"build:transpile:watch": "rollup -c rollup.npm.config.js --watch",
"build:types:watch": "tsc -p tsconfig.types.json --watch",
Expand Down
2 changes: 1 addition & 1 deletion packages/integration-shims/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"build:watch": "run-p build:transpile:watch build:types:watch",
"build:dev:watch": "run-p build:watch",
"build:transpile:watch": "yarn build:transpile --watch",
"build:types:watch": "yarn build:types --watch",
"build:types:watch": "tsc -p tsconfig.types.json --watch",
"clean": "rimraf build",
"fix": "run-s fix:eslint fix:prettier",
"fix:eslint": "eslint . --format stylish --fix",
Expand Down
12 changes: 10 additions & 2 deletions packages/nextjs/test/run-integration-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,21 @@ for NEXTJS_VERSION in 10 11 12 13; do
rm -rf node_modules .next .env.local 2>/dev/null || true

echo "[nextjs@$NEXTJS_VERSION] Installing dependencies..."

# set the desired version of next long enough to run yarn, and then restore the old version (doing the restoration now
# rather than during overall cleanup lets us look for "latest" in every loop)

if [ "$NEXTJS_VERSION" -eq "13" ]; then
SPECIFIC_NEXT_VERSION="13.4.9"
else
SPECIFIC_NEXT_VERSION="$NEXTJS_VERSION.x"
fi

cp package.json package.json.bak
if [[ $(uname) == "Darwin" ]]; then
sed -i "" /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
sed -i "" /"next.*latest"/s/latest/"${SPECIFIC_NEXT_VERSION}"/ package.json
else
sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json
sed -i /"next.*latest"/s/latest/"${SPECIFIC_NEXT_VERSION}"/ package.json
fi

# Next.js v13 requires React 18.2.0
Expand Down
26 changes: 21 additions & 5 deletions packages/opentelemetry-node/src/spanprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import type { DynamicSamplingContext, Span as SentrySpan, TraceparentData, Trans
import { isString, logger } from '@sentry/utils';

import { SENTRY_DYNAMIC_SAMPLING_CONTEXT_KEY, SENTRY_TRACE_PARENT_CONTEXT_KEY } from './constants';
import { isSentryRequestSpan } from './utils/is-sentry-request';
import { mapOtelStatus } from './utils/map-otel-status';
import { parseSpanDescription } from './utils/parse-otel-span-description';
import { isSentryRequestSpan } from './utils/isSentryRequest';
import { mapOtelStatus } from './utils/mapOtelStatus';
import { parseSpanDescription } from './utils/parseOtelSpanDescription';

export const SENTRY_SPAN_PROCESSOR_MAP: Map<SentrySpan['spanId'], SentrySpan> = new Map<
SentrySpan['spanId'],
Expand Down Expand Up @@ -207,6 +207,8 @@ function getTraceData(otelSpan: OtelSpan, parentContext: Context): Partial<Trans
function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): void {
const { attributes, kind } = otelSpan;

const { op, description, data } = parseSpanDescription(otelSpan);

sentrySpan.setStatus(mapOtelStatus(otelSpan));
sentrySpan.setData('otel.kind', SpanKind[kind]);

Expand All @@ -215,20 +217,34 @@ function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): voi
sentrySpan.setData(prop, value);
});

const { op, description } = parseSpanDescription(otelSpan);
if (data) {
Object.keys(data).forEach(prop => {
const value = data[prop];
sentrySpan.setData(prop, value);
});
}

sentrySpan.op = op;
sentrySpan.description = description;
}

function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelSpan): void {
const { op, description, source, data } = parseSpanDescription(otelSpan);

transaction.setContext('otel', {
attributes: otelSpan.attributes,
resource: otelSpan.resource.attributes,
});

if (data) {
Object.keys(data).forEach(prop => {
const value = data[prop];
transaction.setData(prop, value);
});
}

transaction.setStatus(mapOtelStatus(otelSpan));

const { op, description, source } = parseSpanDescription(otelSpan);
transaction.op = op;
transaction.setName(description, source);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import type { AttributeValue } from '@opentelemetry/api';
import type { Attributes, AttributeValue } from '@opentelemetry/api';
import { SpanKind } from '@opentelemetry/api';
import type { Span as OtelSpan } from '@opentelemetry/sdk-trace-base';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import type { TransactionSource } from '@sentry/types';
import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils';

interface SpanDescription {
op: string | undefined;
description: string;
source: TransactionSource;
data?: Record<string, string>;
}

/**
Expand Down Expand Up @@ -87,21 +89,77 @@ function descriptionForHttpMethod(otelSpan: OtelSpan, httpMethod: AttributeValue
break;
}

const httpTarget = attributes[SemanticAttributes.HTTP_TARGET];
const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE];
const { urlPath, url, query, fragment } = getSanitizedUrl(attributes, kind);

// Ex. /api/users
const httpPath = httpRoute || httpTarget;

if (!httpPath) {
if (!urlPath) {
return { op: opParts.join('.'), description: name, source: 'custom' };
}

// Ex. description="GET /api/users".
const description = `${httpMethod} ${httpPath}`;
const description = `${httpMethod} ${urlPath}`;

// If `httpPath` is a root path, then we can categorize the transaction source as route.
const source: TransactionSource = httpRoute || httpPath === '/' ? 'route' : 'url';
const source: TransactionSource = httpRoute || urlPath === '/' ? 'route' : 'url';

const data: Record<string, string> = {};

if (url) {
data.url = url;
}
if (query) {
data['http.query'] = query;
}
if (fragment) {
data['http.fragment'] = fragment;
}

return {
op: opParts.join('.'),
description,
source,
data,
};
}

/** Exported for tests only */
export function getSanitizedUrl(
attributes: Attributes,
kind: SpanKind,
): {
url: string | undefined;
urlPath: string | undefined;
query: string | undefined;
fragment: string | undefined;
} {
// This is the relative path of the URL, e.g. /sub
const httpTarget = attributes[SemanticAttributes.HTTP_TARGET];
// This is the full URL, including host & query params etc., e.g. https://example.com/sub?foo=bar
const httpUrl = attributes[SemanticAttributes.HTTP_URL];
// This is the normalized route name - may not always be available!
const httpRoute = attributes[SemanticAttributes.HTTP_ROUTE];

const parsedUrl = typeof httpUrl === 'string' ? parseUrl(httpUrl) : undefined;
const url = parsedUrl ? getSanitizedUrlString(parsedUrl) : undefined;
const query = parsedUrl && parsedUrl.search ? parsedUrl.search : undefined;
const fragment = parsedUrl && parsedUrl.hash ? parsedUrl.hash : undefined;

if (typeof httpRoute === 'string') {
return { urlPath: httpRoute, url, query, fragment };
}

if (kind === SpanKind.SERVER && typeof httpTarget === 'string') {
return { urlPath: stripUrlQueryAndFragment(httpTarget), url, query, fragment };
}

if (parsedUrl) {
return { urlPath: url, url, query, fragment };
}

// fall back to target even for client spans, if no URL is present
if (typeof httpTarget === 'string') {
return { urlPath: stripUrlQueryAndFragment(httpTarget), url, query, fragment };
}

return { op: opParts.join('.'), description, source };
return { urlPath: undefined, url, query, fragment };
}
48 changes: 47 additions & 1 deletion packages/opentelemetry-node/test/spanprocessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,19 @@ describe('SentrySpanProcessor', () => {
child.setAttribute(SemanticAttributes.HTTP_METHOD, 'GET');
child.setAttribute(SemanticAttributes.HTTP_ROUTE, '/my/route/{id}');
child.setAttribute(SemanticAttributes.HTTP_TARGET, '/my/route/123');
child.setAttribute(SemanticAttributes.HTTP_URL, 'http://example.com/my/route/123');

child.end();

expect(sentrySpan?.description).toBe('GET /my/route/{id}');
expect(sentrySpan?.data).toEqual({
'http.method': 'GET',
'http.route': '/my/route/{id}',
'http.target': '/my/route/123',
'http.url': 'http://example.com/my/route/123',
'otel.kind': 'INTERNAL',
url: 'http://example.com/my/route/123',
});

parentOtelSpan.end();
});
Expand All @@ -453,10 +462,47 @@ describe('SentrySpanProcessor', () => {

child.setAttribute(SemanticAttributes.HTTP_METHOD, 'GET');
child.setAttribute(SemanticAttributes.HTTP_TARGET, '/my/route/123');
child.setAttribute(SemanticAttributes.HTTP_URL, 'http://example.com/my/route/123');

child.end();

expect(sentrySpan?.description).toBe('GET /my/route/123');
expect(sentrySpan?.description).toBe('GET http://example.com/my/route/123');
expect(sentrySpan?.data).toEqual({
'http.method': 'GET',
'http.target': '/my/route/123',
'http.url': 'http://example.com/my/route/123',
'otel.kind': 'INTERNAL',
url: 'http://example.com/my/route/123',
});

parentOtelSpan.end();
});
});
});

it('Adds query & hash data based on HTTP_URL', async () => {
const tracer = provider.getTracer('default');

tracer.startActiveSpan('GET /users', parentOtelSpan => {
tracer.startActiveSpan('HTTP GET', child => {
const sentrySpan = getSpanForOtelSpan(child);

child.setAttribute(SemanticAttributes.HTTP_METHOD, 'GET');
child.setAttribute(SemanticAttributes.HTTP_TARGET, '/my/route/123');
child.setAttribute(SemanticAttributes.HTTP_URL, 'http://example.com/my/route/123?what=123#myHash');

child.end();

expect(sentrySpan?.description).toBe('GET http://example.com/my/route/123');
expect(sentrySpan?.data).toEqual({
'http.method': 'GET',
'http.target': '/my/route/123',
'http.url': 'http://example.com/my/route/123?what=123#myHash',
'otel.kind': 'INTERNAL',
url: 'http://example.com/my/route/123',
'http.query': '?what=123',
'http.fragment': '#myHash',
});

parentOtelSpan.end();
});
Expand Down
Loading

0 comments on commit 377d74f

Please sign in to comment.