Skip to content

Commit

Permalink
fix: add secureConnectionStart to https only (#3879)
Browse files Browse the repository at this point in the history
Co-authored-by: t2t2 <taavot@gmail.com>
Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
  • Loading branch information
3 people authored Jul 5, 2023
1 parent dccd906 commit ea160d9
Show file tree
Hide file tree
Showing 5 changed files with 188 additions and 279 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :bug: (Bug Fix)

* fix(sdk-trace-web): add secureConnectionStart to https only [#3879](https://github.com/open-telemetry/opentelemetry-js/pull/3879) @Abinet18
* fix(http-instrumentation): stop listening to `request`'s `close` event once it has emitted `response` [#3625](https://github.com/open-telemetry/opentelemetry-js/pull/3625) @SimenB
* fix(sdk-node): fix initialization in bundled environments by not loading @opentelemetry/exporter-jaeger [#3739](https://github.com/open-telemetry/opentelemetry-js/pull/3739) @pichlermarc

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,19 @@ function createFakePerformanceObs(url: string) {
return FakePerfObs;
}

function testForCorrectEvents(
events: tracing.TimedEvent[],
eventNames: string[]
) {
for (let i = 0; i < events.length; i++) {
assert.strictEqual(
events[i].name,
eventNames[i],
`event ${eventNames[i]} is not defined`
);
}
}

describe('fetch', () => {
let contextManager: ZoneContextManager;
let lastResponse: any | undefined;
Expand All @@ -152,6 +165,7 @@ describe('fetch', () => {
let fetchInstrumentation: FetchInstrumentation;

const url = 'http://localhost:8090/get';
const secureUrl = 'https://localhost:8090/get';
const badUrl = 'http://foo.bar.com/get';

const clearData = () => {
Expand Down Expand Up @@ -399,53 +413,17 @@ describe('fetch', () => {
it('span should have correct events', () => {
const span: tracing.ReadableSpan = exportSpy.args[1][0][0];
const events = span.events;
assert.strictEqual(events.length, 9, 'number of events is wrong');

assert.strictEqual(
events[0].name,
assert.strictEqual(events.length, 8, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
`event ${PTN.FETCH_START} is not defined`
);
assert.strictEqual(
events[1].name,
PTN.DOMAIN_LOOKUP_START,
`event ${PTN.DOMAIN_LOOKUP_START} is not defined`
);
assert.strictEqual(
events[2].name,
PTN.DOMAIN_LOOKUP_END,
`event ${PTN.DOMAIN_LOOKUP_END} is not defined`
);
assert.strictEqual(
events[3].name,
PTN.CONNECT_START,
`event ${PTN.CONNECT_START} is not defined`
);
assert.strictEqual(
events[4].name,
PTN.SECURE_CONNECTION_START,
`event ${PTN.SECURE_CONNECTION_START} is not defined`
);
assert.strictEqual(
events[5].name,
PTN.CONNECT_END,
`event ${PTN.CONNECT_END} is not defined`
);
assert.strictEqual(
events[6].name,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
assert.strictEqual(
events[7].name,
PTN.RESPONSE_START,
`event ${PTN.RESPONSE_START} is not defined`
);
assert.strictEqual(
events[8].name,
PTN.RESPONSE_END,
`event ${PTN.RESPONSE_END} is not defined`
);
]);
});

it('should create a span for preflight request', () => {
Expand Down Expand Up @@ -479,53 +457,17 @@ describe('fetch', () => {
it('preflight request span should have correct events', () => {
const span: tracing.ReadableSpan = exportSpy.args[0][0][0];
const events = span.events;
assert.strictEqual(events.length, 9, 'number of events is wrong');

assert.strictEqual(
events[0].name,
assert.strictEqual(events.length, 8, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
`event ${PTN.FETCH_START} is not defined`
);
assert.strictEqual(
events[1].name,
PTN.DOMAIN_LOOKUP_START,
`event ${PTN.DOMAIN_LOOKUP_START} is not defined`
);
assert.strictEqual(
events[2].name,
PTN.DOMAIN_LOOKUP_END,
`event ${PTN.DOMAIN_LOOKUP_END} is not defined`
);
assert.strictEqual(
events[3].name,
PTN.CONNECT_START,
`event ${PTN.CONNECT_START} is not defined`
);
assert.strictEqual(
events[4].name,
PTN.SECURE_CONNECTION_START,
`event ${PTN.SECURE_CONNECTION_START} is not defined`
);
assert.strictEqual(
events[5].name,
PTN.CONNECT_END,
`event ${PTN.CONNECT_END} is not defined`
);
assert.strictEqual(
events[6].name,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
assert.strictEqual(
events[7].name,
PTN.RESPONSE_START,
`event ${PTN.RESPONSE_START} is not defined`
);
assert.strictEqual(
events[8].name,
PTN.RESPONSE_END,
`event ${PTN.RESPONSE_END} is not defined`
);
]);
});

it('should set trace headers', () => {
Expand Down Expand Up @@ -639,6 +581,51 @@ describe('fetch', () => {
});
});

describe('when request is secure and successful', () => {
beforeEach(async () => {
const propagateTraceHeaderCorsUrls = [secureUrl];
await prepareData(secureUrl, { propagateTraceHeaderCorsUrls });
});

afterEach(() => {
clearData();
});

it('span should have correct events', () => {
const span: tracing.ReadableSpan = exportSpy.args[1][0][0];
const events = span.events;
assert.strictEqual(events.length, 9, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
PTN.DOMAIN_LOOKUP_START,
PTN.DOMAIN_LOOKUP_END,
PTN.CONNECT_START,
PTN.SECURE_CONNECTION_START,
PTN.CONNECT_END,
PTN.REQUEST_START,
PTN.RESPONSE_START,
PTN.RESPONSE_END,
]);
});

it('preflight request span should have correct events', () => {
const span: tracing.ReadableSpan = exportSpy.args[0][0][0];
const events = span.events;
assert.strictEqual(events.length, 9, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
PTN.DOMAIN_LOOKUP_START,
PTN.DOMAIN_LOOKUP_END,
PTN.CONNECT_START,
PTN.SECURE_CONNECTION_START,
PTN.CONNECT_END,
PTN.REQUEST_START,
PTN.RESPONSE_START,
PTN.RESPONSE_END,
]);
});
});

describe('applyCustomAttributesOnSpan option', () => {
const prepare = async (
url: string,
Expand Down Expand Up @@ -815,13 +802,18 @@ describe('fetch', () => {
`Wrong number of spans: ${exportSpy.args.length}`
);

assert.strictEqual(events.length, 9, 'number of events is wrong');
assert.strictEqual(events.length, 8, 'number of events is wrong');

assert.strictEqual(
events[6].name,
testForCorrectEvents(events, [
PTN.FETCH_START,
PTN.DOMAIN_LOOKUP_START,
PTN.DOMAIN_LOOKUP_END,
PTN.CONNECT_START,
PTN.CONNECT_END,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
PTN.RESPONSE_START,
PTN.RESPONSE_END,
]);
});
});

Expand All @@ -844,12 +836,17 @@ describe('fetch', () => {
`Wrong number of spans: ${exportSpy.args.length}`
);

assert.strictEqual(events.length, 9, 'number of events is wrong');
assert.strictEqual(
events[6].name,
assert.strictEqual(events.length, 8, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
PTN.DOMAIN_LOOKUP_START,
PTN.DOMAIN_LOOKUP_END,
PTN.CONNECT_START,
PTN.CONNECT_END,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
PTN.RESPONSE_START,
PTN.RESPONSE_END,
]);
});

it('should have an absolute http.url attribute', () => {
Expand Down Expand Up @@ -882,12 +879,17 @@ describe('fetch', () => {
2,
`Wrong number of spans: ${exportSpy.args.length}`
);
assert.strictEqual(events.length, 9, 'number of events is wrong');
assert.strictEqual(
events[6].name,
assert.strictEqual(events.length, 8, 'number of events is wrong');
testForCorrectEvents(events, [
PTN.FETCH_START,
PTN.DOMAIN_LOOKUP_START,
PTN.DOMAIN_LOOKUP_END,
PTN.CONNECT_START,
PTN.CONNECT_END,
PTN.REQUEST_START,
`event ${PTN.REQUEST_START} is not defined`
);
PTN.RESPONSE_START,
PTN.RESPONSE_END,
]);
});
});

Expand Down
Loading

0 comments on commit ea160d9

Please sign in to comment.