Skip to content

Commit

Permalink
fix(connect): Skip update HTTP's span name and update RpcMetadata's r…
Browse files Browse the repository at this point in the history
…oute instead (#1534)

Co-authored-by: Gerhard Stöbich <deb2001-github@yahoo.de>
  • Loading branch information
chigia001 and Flarna committed Jun 28, 2023
1 parent f7c4324 commit 8499b16
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { context, diag, Span, SpanOptions } from '@opentelemetry/api';
import { getRPCMetadata, RPCType } from '@opentelemetry/core';
import type { HandleFunction, NextFunction, Server } from 'connect';
import type { IncomingMessage, ServerResponse } from 'http';
import type { ServerResponse } from 'http';
import {
AttributeNames,
ConnectNames,
Expand Down Expand Up @@ -120,16 +120,13 @@ export class ConnectInstrumentation extends InstrumentationBase<Server> {
if (!instrumentation.isEnabled()) {
return (middleWare as any).apply(this, arguments);
}
const [reqArgIdx, resArgIdx, nextArgIdx] = isErrorMiddleware
? [1, 2, 3]
: [0, 1, 2];
const req = arguments[reqArgIdx] as IncomingMessage;
const [resArgIdx, nextArgIdx] = isErrorMiddleware ? [2, 3] : [1, 2];
const res = arguments[resArgIdx] as ServerResponse;
const next = arguments[nextArgIdx] as NextFunction;

const rpcMetadata = getRPCMetadata(context.active());
if (routeName && rpcMetadata?.type === RPCType.HTTP) {
rpcMetadata.span.updateName(`${req.method} ${routeName || '/'}`);
rpcMetadata.route = routeName;
}
let spanName = '';
if (routeName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import * as assert from 'assert';

import { context, trace } from '@opentelemetry/api';
import { RPCType, setRPCMetadata } from '@opentelemetry/core';
import { RPCType, setRPCMetadata, RPCMetadata } from '@opentelemetry/core';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
Expand Down Expand Up @@ -186,7 +186,7 @@ describe('connect', () => {
assert.strictEqual(span.name, 'request handler - /foo');
});

it('should change name for parent http route', async () => {
it('should not change name for parent http route ', async () => {
const rootSpan = tracer.startSpan('root span');
app.use((req, res, next) => {
const rpcMetadata = { type: RPCType.HTTP, span: rootSpan };
Expand All @@ -206,11 +206,37 @@ describe('connect', () => {
await httpRequest.get(`http://localhost:${PORT}/foo`);
rootSpan.end();

const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 3);
const changedRootSpan = spans[2];
assert.strictEqual(changedRootSpan.name, 'root span');
});

it('should mutate route value of RpcMetadata', async () => {
const rootSpan = tracer.startSpan('root span');
const rpcMetadata: RPCMetadata = { type: RPCType.HTTP, span: rootSpan };
app.use((req, res, next) => {
return context.with(
setRPCMetadata(
trace.setSpan(context.active(), rootSpan),
rpcMetadata
),
next
);
});

app.use('/foo', (req, res, next) => {
next();
});

await httpRequest.get(`http://localhost:${PORT}/foo`);
rootSpan.end();

const spans = memoryExporter.getFinishedSpans();
assert.strictEqual(spans.length, 3);
const changedRootSpan = spans[2];
const span = spans[0];
assert.strictEqual(changedRootSpan.name, 'GET /foo');
assert.strictEqual(rpcMetadata.route, '/foo');
assert.strictEqual(span.name, 'request handler - /foo');
assert.strictEqual(
span.parentSpanId,
Expand Down

0 comments on commit 8499b16

Please sign in to comment.