Skip to content

Commit

Permalink
feat(tracing): auto flush BatchSpanProcessor on browser (#2243)
Browse files Browse the repository at this point in the history
  • Loading branch information
kkruk-sumo authored Jun 16, 2021
1 parent a3b7738 commit 23db7f0
Show file tree
Hide file tree
Showing 24 changed files with 285 additions and 27 deletions.
4 changes: 3 additions & 1 deletion packages/opentelemetry-tracing/karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const karmaBaseConfig = require('../../karma.base');

module.exports = (config) => {
config.set(Object.assign({}, karmaBaseConfig, {
webpack: karmaWebpackConfig
webpack: karmaWebpackConfig,
files: ['test/browser/index-webpack.ts'],
preprocessors: { 'test/browser/index-webpack.ts': ['webpack'] }
}))
};
2 changes: 1 addition & 1 deletion packages/opentelemetry-tracing/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"scripts": {
"compile": "tsc --build tsconfig.json tsconfig.esm.json",
"clean": "tsc --build --clean tsconfig.json tsconfig.esm.json",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/index-webpack.ts'",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'",
"test:browser": "nyc karma start --single-run",
"tdd": "npm run tdd:node",
"tdd:node": "npm run test -- --watch-extensions ts --watch",
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-tracing/src/BasicTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { SDKRegistrationConfig, TracerConfig } from './types';
// eslint-disable-next-line @typescript-eslint/no-var-requires
const merge = require('lodash.merge');
import { SpanExporter } from './export/SpanExporter';
import { BatchSpanProcessor } from './export/BatchSpanProcessor';
import { BatchSpanProcessor } from './platform';

export type PROPAGATOR_FACTORY = () => TextMapPropagator;
export type EXPORTER_FACTORY = () => SpanExporter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { SpanExporter } from './SpanExporter';
* Implementation of the {@link SpanProcessor} that batches spans exported by
* the SDK then pushes them to the exporter pipeline.
*/
export class BatchSpanProcessor implements SpanProcessor {
export abstract class BatchSpanProcessorBase<T extends BufferConfig> implements SpanProcessor {
private readonly _maxExportBatchSize: number;
private readonly _maxQueueSize: number;
private readonly _scheduledDelayMillis: number;
Expand All @@ -43,23 +43,23 @@ export class BatchSpanProcessor implements SpanProcessor {
private _isShutdown = false;
private _shuttingDownPromise: Promise<void> = Promise.resolve();

constructor(private readonly _exporter: SpanExporter, config?: BufferConfig) {
constructor(private readonly _exporter: SpanExporter, config?: T) {
const env = getEnv();
this._maxExportBatchSize =
typeof config?.maxExportBatchSize === 'number'
? config.maxExportBatchSize
: env.OTEL_BSP_MAX_EXPORT_BATCH_SIZE;
this._maxQueueSize =
typeof config?.maxQueueSize === 'number'
? config?.maxQueueSize
? config.maxQueueSize
: env.OTEL_BSP_MAX_QUEUE_SIZE;
this._scheduledDelayMillis =
typeof config?.scheduledDelayMillis === 'number'
? config?.scheduledDelayMillis
? config.scheduledDelayMillis
: env.OTEL_BSP_SCHEDULE_DELAY;
this._exportTimeoutMillis =
typeof config?.exportTimeoutMillis === 'number'
? config?.exportTimeoutMillis
? config.exportTimeoutMillis
: env.OTEL_BSP_EXPORT_TIMEOUT;
}

Expand Down Expand Up @@ -87,6 +87,9 @@ export class BatchSpanProcessor implements SpanProcessor {
this._isShutdown = true;
this._shuttingDownPromise = new Promise((resolve, reject) => {
Promise.resolve()
.then(() => {
return this.onShutdown();
})
.then(() => {
return this._flushAll();
})
Expand Down Expand Up @@ -190,4 +193,6 @@ export class BatchSpanProcessor implements SpanProcessor {
this._timer = undefined;
}
}

protected abstract onShutdown(): void;
}
2 changes: 1 addition & 1 deletion packages/opentelemetry-tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

export * from './Tracer';
export * from './BasicTracerProvider';
export * from './platform';
export * from './export/ConsoleSpanExporter';
export * from './export/BatchSpanProcessor';
export * from './export/InMemorySpanExporter';
export * from './export/ReadableSpan';
export * from './export/SimpleSpanProcessor';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { BatchSpanProcessorBase } from '../../../export/BatchSpanProcessorBase';
import { SpanExporter } from '../../../export/SpanExporter';
import { BatchSpanProcessorBrowserConfig } from '../../../types';

export class BatchSpanProcessor extends BatchSpanProcessorBase<BatchSpanProcessorBrowserConfig> {
private _visibilityChangeListener?: () => void
private _pageHideListener?: () => void

constructor(_exporter: SpanExporter, config?: BatchSpanProcessorBrowserConfig) {
super(_exporter, config)
this.onInit(config)
}

private onInit(config?: BatchSpanProcessorBrowserConfig): void {
if (config?.disableAutoFlushOnDocumentHide !== true && document != null) {
this._visibilityChangeListener = () => {
if (document.visibilityState === 'hidden') {
void this.forceFlush();
}
}
this._pageHideListener = () => {
void this.forceFlush()
}
document.addEventListener('visibilitychange', this._visibilityChangeListener);

// use 'pagehide' event as a fallback for Safari; see https://bugs.webkit.org/show_bug.cgi?id=116769
document.addEventListener('pagehide', this._pageHideListener);
}
}

protected onShutdown(): void {
if (this._visibilityChangeListener) {
document.removeEventListener('visibilitychange', this._visibilityChangeListener);
}
if (this._pageHideListener) {
document.removeEventListener('pagehide', this._pageHideListener);
}
}
}
17 changes: 17 additions & 0 deletions packages/opentelemetry-tracing/src/platform/browser/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from './export/BatchSpanProcessor';
17 changes: 17 additions & 0 deletions packages/opentelemetry-tracing/src/platform/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from './node';
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { BatchSpanProcessorBase } from '../../../export/BatchSpanProcessorBase';
import { BufferConfig } from '../../../types';

export class BatchSpanProcessor extends BatchSpanProcessorBase<BufferConfig> {
protected onShutdown(): void {}

This comment has been minimized.

Copy link
@temirrr

temirrr Sep 10, 2024

@kkruk-sumo hi, could you explain why this onShutdown override makes it do nothing instead of using the base class behaviour?

IIUC my code doesn't send unflushed spans on shutdown because of this override. I guess I should just do my own implementation as one way to work around that

}
17 changes: 17 additions & 0 deletions packages/opentelemetry-tracing/src/platform/node/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

export * from './export/BatchSpanProcessor';
7 changes: 7 additions & 0 deletions packages/opentelemetry-tracing/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,10 @@ export interface BufferConfig {
* The default value is 2048. */
maxQueueSize?: number;
}

/** Interface configuration for BatchSpanProcessor on browser */
export interface BatchSpanProcessorBrowserConfig extends BufferConfig {
/** Disable flush when a user navigates to a new page, closes the tab or the browser, or,
* on mobile, switches to a different app. Auto flush is enabled by default. */
disableAutoFlushOnDocumentHide?: boolean;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as assert from 'assert';
import * as sinon from 'sinon';
import { SpanExporter } from '../../../src';
import { BatchSpanProcessor } from '../../../src/platform/browser/export/BatchSpanProcessor';
import { TestTracingSpanExporter } from '../../common/export/TestTracingSpanExporter';

describe('BatchSpanProcessor - web', () => {
let visibilityState: VisibilityState = 'visible';
let exporter: SpanExporter
let processor: BatchSpanProcessor;
let forceFlushSpy: sinon.SinonStub;
let visibilityChangeEvent: Event;
let pageHideEvent: Event

beforeEach(() => {
sinon.replaceGetter(document, 'visibilityState', () => visibilityState);
visibilityState = 'visible';
exporter = new TestTracingSpanExporter();
processor = new BatchSpanProcessor(exporter, {});
forceFlushSpy = sinon.stub(processor, 'forceFlush');
visibilityChangeEvent = new Event('visibilitychange');
pageHideEvent = new Event('pagehide');
});

afterEach(async () => {
sinon.restore();
});

describe('when document becomes hidden', () => {
const testDocumentHide = (hideDocument: () => void) => {
it('should force flush spans', () => {
assert.strictEqual(forceFlushSpy.callCount, 0);
hideDocument()
assert.strictEqual(forceFlushSpy.callCount, 1);
});

describe('AND shutdown has been called', () => {
it('should NOT force flush spans', async () => {
assert.strictEqual(forceFlushSpy.callCount, 0);
await processor.shutdown();
hideDocument()
assert.strictEqual(forceFlushSpy.callCount, 0);
});
})

describe('AND disableAutoFlushOnDocumentHide configuration option', () => {
it('set to false should force flush spans', () => {
processor = new BatchSpanProcessor(exporter, { disableAutoFlushOnDocumentHide: false });
forceFlushSpy = sinon.stub(processor, 'forceFlush');
assert.strictEqual(forceFlushSpy.callCount, 0);
hideDocument()
assert.strictEqual(forceFlushSpy.callCount, 1);
})

it('set to true should NOT force flush spans', () => {
processor = new BatchSpanProcessor(exporter, { disableAutoFlushOnDocumentHide: true });
forceFlushSpy = sinon.stub(processor, 'forceFlush');
assert.strictEqual(forceFlushSpy.callCount, 0);
hideDocument()
assert.strictEqual(forceFlushSpy.callCount, 0);
})
})
}

describe('by the visibilitychange event', () => {
testDocumentHide(() => {
visibilityState = 'hidden';
document.dispatchEvent(visibilityChangeEvent);
})
})

describe('by the pagehide event', () => {
testDocumentHide(() => {
document.dispatchEvent(pageHideEvent);
})
})
})

describe('when document becomes visible', () => {
it('should NOT force flush spans', () => {
assert.strictEqual(forceFlushSpy.callCount, 0);
document.dispatchEvent(visibilityChangeEvent);
assert.strictEqual(forceFlushSpy.callCount, 0);
});
})
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
const testsContext = require.context('.', true, /test$/);
const testsContext = require.context('../browser', true, /test$/);
testsContext.keys().forEach(testsContext);

const testsContextCommon = require.context('../common', true, /test$/);
testsContextCommon.keys().forEach(testsContextCommon);

const srcContext = require.context('.', true, /src$/);
srcContext.keys().forEach(srcContext);
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import {
InMemorySpanExporter,
SpanExporter,
BatchSpanProcessor,
} from '../src';
} from '../../src';

describe('BasicTracerProvider', () => {
let removeEvent: Function | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import {
SimpleSpanProcessor,
Span,
SpanProcessor,
} from '../src';
} from '../../src';
import {
setGlobalErrorHandler,
loggingErrorHandler,
} from '@opentelemetry/core';
import { MultiSpanProcessor } from '../src/MultiSpanProcessor';
import { MultiSpanProcessor } from '../../src/MultiSpanProcessor';

class TestProcessor implements SpanProcessor {
spans: Span[] = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from '@opentelemetry/core';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import { BasicTracerProvider, Span, SpanProcessor } from '../src';
import { BasicTracerProvider, Span, SpanProcessor } from '../../src';

const performanceTimeOrigin = hrTime();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
suppressTracing
} from '@opentelemetry/core';
import * as assert from 'assert';
import { BasicTracerProvider, Span, Tracer } from '../src';
import { BasicTracerProvider, Span, Tracer } from '../../src';
import { TestStackContextManager } from './export/TestStackContextManager';
import * as sinon from 'sinon';

Expand Down
Loading

0 comments on commit 23db7f0

Please sign in to comment.