airbyte-metrics/server: add tags to root span from OAuth handler

[alafanechere]

What

For better error tracking, I want to tag the root span with workspace id and definition id when OAuth handler public methods are called.

How

• Create a ApmTraceUtils.addTagsToRootSpan function to add tags to the root span
• Tag the root span from OAuthHandler

[alafanechere]

I confirm that this changed propagates tags to the root span:

[jdpgrailsdev]

@alafanechere Would you mind adding tests for this new method (and the one that adds an error to the root span) to the ApmTraceUtilsTest class?

[evantahler]

[question] If the framework is wrapping all requests in a span, how would we not have a span? Perhaps if Data dog is disabled?

[jdpgrailsdev]

@evantahler Good catch. This method should check if activeSpan is not null. It will be null when the DataDog Java agent is not present. cc: @alafanechere

[alafanechere]

Thanks for the suggestion. Isn't if activeSpan instanceof MutableSpan already checking a not null by itself?

[alafanechere]

@evantahler MutableSpan is a datadog interface. And Span and GlobalTracer.get().activeSpan() are from opentelemetry. So when datadog is disabled we might not received Span object implementing the MutableSpan interface, hence the importance of this check. Am I right @jdpgrailsdev ?

[jdpgrailsdev]

@alafanechere Correct on both accounts. The instanceof check does protect us from null, as it will resolve to false. MutableSpan is a DataDog only class, so if a customer uses a different APM product that supports OpenTracing, that check protects us from calling methods that don't exist on the object.

[evantahler]

[non-blocking nit] when I've done instrumentation like this in the past, the strategy was to append as much information to the trace / root-span at the top level before getting into any individual controller. This looks like a global rootSpanHelper which intercepts /every/ request (e.g. "request middleware") and:

• if there's a user, addd the user_id
• if there's a workspace, add the workspace_id
• if there are any LaunchDarkly feature flags, add them to the span

Then, in the controller itself, you only have to add new information (the source_id in this case).

[alafanechere]

I like this idea of helper/request middleware. This abstraction could also help us switch tracking backend (sentry/datadog) with a single interface. @evantahler could it fit a bigger project we'd have in Q1?

[alafanechere]

@alafanechere Would you mind adding tests for this new method (and the one that adds an error to the root span) to the ApmTraceUtilsTest class?

Test added 👌