Skip to content

Commit

Permalink
fix(instrumentation-net): make tls span parent of net span (#1342)
Browse files Browse the repository at this point in the history
* fix(instrumentation-net): make tls span parent of net span

* chore: lint fix
  • Loading branch information
blumamir authored Jan 15, 2023
1 parent 714e6c1 commit 1ee197e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
},
"devDependencies": {
"@opentelemetry/api": "^1.3.0",
"@opentelemetry/context-async-hooks": "^1.8.0",
"@opentelemetry/sdk-trace-base": "^1.8.0",
"@opentelemetry/sdk-trace-node": "^1.8.0",
"@types/mocha": "7.0.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { diag, Span, SpanStatusCode } from '@opentelemetry/api';
import { diag, Span, SpanStatusCode, context, trace } from '@opentelemetry/api';
import {
InstrumentationBase,
InstrumentationConfig,
Expand Down Expand Up @@ -112,7 +112,12 @@ export class NetInstrumentation extends InstrumentationBase<Net> {
) {
const tlsSpan = this.tracer.startSpan('tls.connect');

const netSpan = this._startSpan(options, socket);
const netSpan = context.with(
trace.setSpan(context.active(), tlsSpan),
() => {
return this._startSpan(options, socket);
}
);

const otelTlsSpanListener = () => {
const peerCertificate = socket.getPeerCertificate(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
* limitations under the License.
*/

import { SpanStatusCode } from '@opentelemetry/api';
import { SpanStatusCode, context } from '@opentelemetry/api';
import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import * as assert from 'assert';
import * as tls from 'tls';
import { NetInstrumentation } from '../src';
Expand Down Expand Up @@ -48,12 +49,16 @@ function getTLSSpans() {

describe('NetInstrumentation', () => {
let instrumentation: NetInstrumentation;
let contextManager: AsyncHooksContextManager;

let tlsServer: tls.Server;
let tlsSocket: tls.TLSSocket;

before(() => {
instrumentation = new NetInstrumentation();
instrumentation.setTracerProvider(provider);
contextManager = new AsyncHooksContextManager().enable();
context.setGlobalContextManager(contextManager.enable());
require('net');
});

Expand Down
12 changes: 12 additions & 0 deletions plugins/node/opentelemetry-instrumentation-net/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export function assertTLSSpan(
{ netSpan, tlsSpan }: { netSpan: ReadableSpan; tlsSpan: ReadableSpan },
socket: Socket
) {
assertParentChild(tlsSpan, netSpan);
assertSpanKind(netSpan);
assertAttrib(
netSpan,
Expand Down Expand Up @@ -108,6 +109,17 @@ export function assertAttrib(span: ReadableSpan, attrib: string, value: any) {
assert.strictEqual(span.attributes[attrib], value);
}

export function assertParentChild(
parentSpan: ReadableSpan,
childSpan: ReadableSpan
) {
assert.strictEqual(
childSpan.spanContext().traceId,
parentSpan.spanContext().traceId
);
assert.strictEqual(childSpan.parentSpanId, parentSpan.spanContext().spanId);
}

export const TLS_SERVER_CERT = fs
.readFileSync(path.resolve(__dirname, './fixtures/tls.crt'))
.toString();
Expand Down

0 comments on commit 1ee197e

Please sign in to comment.