Add support for ignoring requests in FastifyOtelInstrumentation

[jeffrey-peterson-vanna]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Add support for ignoring OTEL instrumentation based on a user provided hook. This would mimic the configuration of @opentelemetry/instrumentation-http where the instrumentation constructor options includes an ignoreIncomingRequestHook option.

export interface HttpInstrumentationConfig extends InstrumentationConfig {
  /** Not trace all incoming requests that matched with custom function */
  ignoreIncomingRequestHook?: IgnoreIncomingRequestFunction;
  ...
}

export interface IgnoreIncomingRequestFunction {
  (request: IncomingMessage): boolean;
}

Additionally, it would be preferred if the ignoreIncomingRequestHook was invoked during onResponse such that the reply could be provided to the hook as well (e.g. ignore healthchecks with response 200). Implementing this would require the active span for the request lifecycle to be discarded, but I'm unsure about technical feasibility of this as the context and tracing docs don't explicitly include this (as far as I could tell).

Motivation

This would be very helpful for reducing unneeded traces/spans for routes such as health checks or other chatty endpoints. Most OpenTelemetry aggregation vendors charge per trace/span so reducing instrumentation noise is financially important.

Example

new FastifyOtelInstrumentation({
  ignoreIncomingRequestHook: (request): boolean => {
    return request.url === HEALTH_CHECK_URL;
  }
})

or

new FastifyOtelInstrumentation({
  ignoreIncomingRequestHook: (request, reply): boolean => {
    return request.url === HEALTH_CHECK_URL && reply.statusCode === 200;
  }
})

[jsumners]

Isn't the stated use case supported by using a different encapsulation context for the status routes?

[jeffrey-peterson-vanna]

Thanks for the response!

If one was to refactor their server, I believe they could construct encapsulation contexts that subdivide the server into instrumented vs non-instrumented routes. So technically yes, but I think that misses the spirit of the request.

For example take the server in the encapsulation docs, how would one ignore the route '/three' in grandchildServer without refactoring?

[metcoder95]

I believe that the path that @jsumners advices it is the recommended way to go. Tho I can see the chance that are use-cases that cannot be fully covered by the encapsulation feature.

I can see a hook like onRequest be able to decide discard tracing and extend attributes of requests depending on its properties by returning true or false. But having the reply there of course is not possible.

An hypothetic onResponse sadly cannot drop the spans made at that moment as they were already recording by then and for instance childs has been made already, then is not feasible.
Tho I can see a use case for extending the attributes of the span depending on the response sent to the client.

[jsumners]

I was explicitly calling out the stated use case as solved. There are definitely cases that are not solved. But I'm not convinced they should be solved in this specific plugin. Thinking about OTEL specific solutions, I would recommend delivering to a local collector that will make decisions on how much data to ship to the final collector service.

[jeffrey-peterson-vanna]

Thank you both for the discussion. For clarity, I used "ignoring health checks" as a simple example use case. I did not intend it to be the only use case.

Using a filter processor in an OTEL collector would solve the use case, given you have the prescribed architecture in place. But even so, the filter processor is limited to filtering based on span attributes only, which may not be comprehensive enough for an arbitrary use case. Additionally, the collector could be maintained by a different group within an organization which makes keeping the filter logic in sync with the code more challenging.

There is precedent for this functionality as well:

In @opentelemetry/instrumentation-http shown in my first message.
In @opentelemetry/instrumentation-express:

export interface ExpressInstrumentationConfig extends InstrumentationConfig {
    /** Ignore specific based on their name */
    ignoreLayers?: IgnoreMatcher[];
    /** Ignore specific layers based on their type */
    ignoreLayersType?: ExpressLayerType[];
    ...

In @opentelemetry/instrumentation-socket.io:

export interface SocketIoInstrumentationConfig extends InstrumentationConfig {
    ...
    /** list of events to ignore tracing on for socket.io emits */
    emitIgnoreEventList?: string[];
    ...
    /** list of events to ignore tracing on for socket.io listeners */
    onIgnoreEventList?: string[];

In @opentelemetry/instrumentation-dns:

export interface DnsInstrumentationConfig extends InstrumentationConfig {
    ignoreHostnames?: IgnoreMatcher | IgnoreMatcher[];
}

I see a valid use case for ignoring requests within the @fastify/otel instrumentation. If this existed today, I would have used it. My intention here is simply to provide end user feedback to improve the codebase. Hope you all have a great day.

[metcoder95]

I see both solutions coexisting, what we have been discussing should definitely be documented as I believe it will bring a lot of value to people using the instrumentation.

Recommending to use the collector to filter out, instead of relying on the instrumentation to do that, as well the possibility of doing it if accessing that architecture is complicated.
That's why marked the issue as enhancement.

PR's are welcome!