Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(instrumentation): add util to execute span customization hook in base class #4663

Merged
merged 20 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c55be5e
feat(instrumentation): hoist span event hook execution to base class
blumamir Apr 27, 2024
a0e2a8d
test: add test for new hook runner
blumamir Apr 28, 2024
d0cd4d5
chore: changelog
blumamir Apr 28, 2024
1c059fe
fix: use event name from arguments
blumamir Apr 28, 2024
c33ba1a
fix: remove unused import
blumamir Apr 28, 2024
944a224
fix: make diag message structual
blumamir Apr 28, 2024
cac3766
Merge branch 'main' into hook-executer
blumamir Apr 29, 2024
fa1a53e
make the private function start with underscore
blumamir Apr 30, 2024
9dc607c
Merge remote-tracking branch 'origin/hook-executer' into hook-executer
blumamir Apr 30, 2024
9f44c05
Merge branch 'main' into hook-executer
blumamir May 1, 2024
d29bda6
Merge remote-tracking branch 'upstream/main' into hook-executer
blumamir May 3, 2024
86fb72a
Merge branch 'hook-executer' of github.com:blumamir/opentelemetry-js …
blumamir May 3, 2024
c2d7518
chore: rename insetrumentation event to span customization hook
blumamir May 3, 2024
e737b7f
chore: update changelog
blumamir May 3, 2024
68c73ba
chore: lint fix
blumamir May 3, 2024
1540713
Update experimental/packages/opentelemetry-instrumentation/src/instru…
blumamir May 6, 2024
9d0daf9
chore: move CHANGELOG to experimental
blumamir May 6, 2024
2035a34
Merge remote-tracking branch 'upstream/main' into hook-executer
blumamir May 6, 2024
73b1308
Merge remote-tracking branch 'origin/hook-executer' into hook-executer
blumamir May 6, 2024
d4e2f38
fix: changelog
blumamir May 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat(instrumentation): add util to execute span customization hook in base class [#4663](https://github.com/open-telemetry/opentelemetry-js/pull/4663) @blumamir
blumamir marked this conversation as resolved.
Show resolved Hide resolved
* feat(instrumentation): generic config type in instrumentation base [#4659](https://github.com/open-telemetry/opentelemetry-js/pull/4659) @blumamir
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed for #4659 which is not released yet

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this message to experimental/CHANGELOG.md

* feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan
* feat(sdk-trace-node): support `xray` Propagator via `OTEL_PROPAGATORS` environment variable [#4602](https://github.com/open-telemetry/opentelemetry-js/pull/4602) @anuraags
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ import {
trace,
Tracer,
TracerProvider,
Span,
} from '@opentelemetry/api';
import { Logger, LoggerProvider, logs } from '@opentelemetry/api-logs';
import * as shimmer from 'shimmer';
import {
InstrumentationModuleDefinition,
Instrumentation,
InstrumentationConfig,
SpanCustomizationHook,
} from './types';

/**
Expand Down Expand Up @@ -178,4 +180,33 @@ export abstract class InstrumentationAbstract<
| InstrumentationModuleDefinition
| InstrumentationModuleDefinition[]
| void;

/**
* Execute span customization hook, if configured, and log any errors.
* Any semantics of the trigger and info are defined by the specific instrumentation.
* @param hookHandler The optional hook handler which the user has configured via instrumentation config\
blumamir marked this conversation as resolved.
Show resolved Hide resolved
* @param triggerName The name of the trigger for executing the hook for logging purposes
* @param span The span to which the hook should be applied
* @param info The info object to be passed to the hook, with useful data the hook may use
*/
protected _runSpanCustomizationHook<SpanCustomizationInfoType>(
hookHandler: SpanCustomizationHook<SpanCustomizationInfoType> | undefined,
triggerName: string,
span: Span,
Flarna marked this conversation as resolved.
Show resolved Hide resolved
info: SpanCustomizationInfoType
) {
if (!hookHandler) {
return;
}

try {
hookHandler(span, info);
} catch (e) {
this._diag.error(
`Error running span customization hook due to exception in handler`,
{ triggerName },
e
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { TracerProvider, MeterProvider } from '@opentelemetry/api';
import { TracerProvider, MeterProvider, Span } from '@opentelemetry/api';
import { LoggerProvider } from '@opentelemetry/api-logs';

/** Interface Instrumentation to apply patch. */
Expand Down Expand Up @@ -135,3 +135,24 @@ export interface InstrumentationModuleDefinition {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
unpatch?: (moduleExports: any, moduleVersion?: string) => void;
}

/**
* SpanCustomizationHook is a common way for instrumentations to expose extension points
* where users can add custom behavior to a span based on info object passed to the hook at different times of the span lifecycle.
* This is an advanced feature, commonly used to add additional or non-spec-compliant attributes to the span,
* capture payloads, modify the span in some way, or carry some other side effect.
*
* The hook is registered with the instrumentation specific config by implementing an handler function with this signature,
* and if the hook is present, it will be called with the span and the event information
* when the event is emitted.
*
* When and under what conditions the hook is called and what data is passed
* in the info argument, is specific to each instrumentation and life-cycle event
* and should be documented where it is used.
*
* Instrumentation may define multiple hooks, for different spans, or different span life-cycle events.
*/
export type SpanCustomizationHook<SpanCustomizationInfoType> = (
span: Span,
info: SpanCustomizationInfoType
) => void;
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
InstrumentationBase,
InstrumentationConfig,
InstrumentationModuleDefinition,
SpanCustomizationHook,
} from '../../src';

import { MeterProvider } from '@opentelemetry/sdk-metrics';
Expand All @@ -36,6 +37,12 @@ class TestInstrumentation extends InstrumentationBase {
override enable() {}
override disable() {}
init() {}

// the runInstrumentationEventHook, so we have to invoke it from the class for testing
testRunHook(hookHandler?: SpanCustomizationHook<any>) {
const span = this.tracer.startSpan('test');
this._runSpanCustomizationHook(hookHandler, 'test', span, {});
}
}

describe('BaseInstrumentation', () => {
Expand Down Expand Up @@ -182,5 +189,30 @@ describe('BaseInstrumentation', () => {

assert.deepStrictEqual(instrumentation.getModuleDefinitions(), []);
});

describe('runInstrumentationEventHook', () => {
it('should call the hook', () => {
const instrumentation = new TestInstrumentation({});
let called = false;
const hook = () => {
called = true;
};
instrumentation.testRunHook(hook);
assert.strictEqual(called, true);
});

it('empty hook should work', () => {
const instrumentation = new TestInstrumentation({});
instrumentation.testRunHook(undefined);
});

it('exception in hook should not crash', () => {
const instrumentation = new TestInstrumentation({});
const hook = () => {
throw new Error('test');
};
instrumentation.testRunHook(hook);
});
});
});
});