Support filtering functions to trace/ignore http(s) requests

[sergioregueira]

Is your feature request related to a problem?

Current implementation of http plugin allows people to filter out requests that they do not want to trace using either ignoreIncomingPaths or ignoreOutgoingUrls properties (see types).

Nevertheless, those filters are based only on request URL; a smart filtering using either a header, param or method is not possible.

Describe the solution you'd like

Plugin should admit, as configuration parameters, two filtering functions that will decide on execution time if the incoming/outgoing request has to be traced or omitted.

interface ProposedParameters {
  incomingFilter?: (req: http.IncomingMessage) => boolean;
  outgoingFilter?: (req: http.RequestOptions) => boolean;
}

Describe alternatives you've considered

The only valid solution to filter out some requests without defining the whole list of ignored URLs is to add the x-opentelemetry-outgoing-request header (#335) , but it was not defined to be used by applications but exporters.

Additional context

The special header that I have just mentioned caused some troubles (#983) because it was not expected by 3rd-party servers. This proposal is not related to adding a new magic header but using the real data that servers already expect.

[dyladan]

The outgoing use-case could be solved by #1040 but with incoming it is already too late.

[sergioregueira]

Despite I am looking forward to that pull request, I think that feature does not solve this scenario. That solution allows people to completely stop tracing from that moment whereas I just want to omit a specific part and continue tracing the rest of it.

For example, imagine a scenario where a web service has to make a HTTP request to a external 3rd-party (for validation, automation, whatever) that I do not want to trace (because it is defined by customer) but what happens after it is important and I have to trace it. Will I be able to do it with that?

[vmarchaud]

I don't think so. I agree that allowing to filter on anything on the http request make sense (and we should have done it from the start)

[dyladan]

I don't think so. I agree that allowing to filter on anything on the http request make sense (and we should have done it from the start)

Sounds good to me

[sergioregueira]

Some questions about it:

• Should it support multiple filtering functions or a single one?
• Would return true mean it has to be traced or omitted?

IMHO, a single one would be enough and return true would trace the request (inclusive filtering).

[sergioregueira]

By the way, feel free to assign me the issue 🙂

[vmarchaud]

@sergioregueira I would be in favor of breaking the backward compatiblity by replacing url by req altogether, adding another option feels like too much and adding a second argument like `(url: string, req: http.IncomingMessage) => boolean) feels weird. Do you have any take @dyladan ?

On the return true, i believe today we true means that the request is ignored so we should keep that.

[dyladan]

Think breaking backwards compatibility is probably fine here. I agree I don't want to have too many options to maintain and the request filter would be able to completely cover the url usecase easily.

[sergioregueira]

On the return true, i believe today we true means that the request is ignored so we should keep that.

That is right, because those parameters use RegExp | string to define the requests that you want to exclude. Nevertheless, in this new approach, I think that we should understand true as please, trace the request to follow the same approach than Array.prototype.filter().

[dyladan]

Without knowing the way the current filter works and without reading any docs, I would assume returning true causes a request to be traced precisely because I would associate it with Array.prototype.filter. Following the principle of least surprise, I think we should mimic Array.prototype.filter or change the name from filter to something else like ignoreRequestHook

[sergioregueira]

It has been a long time since latest message, but I will resume the task this week.

[alexandernanberg]

This looks to be fixed by #2704, would be great if it could be published!

[dyladan]

I am in the middle of creating the release PR for our stable packages right now and the experimental packages (including that) will follow very soon

[alexandernanberg]

@dyladan That is awesome, thanks! 🎉

[tobiasmuehl]

This has been implemented since, the issue can be closed