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

Custom ObservationHandler beans need to be registered after infrastructure handlers #34399

Closed
Tracked by #35776
ttddyy opened this issue Feb 27, 2023 · 1 comment
Closed
Tracked by #35776
Labels
theme: observability Issues related to observability type: enhancement A general enhancement
Milestone

Comments

@ttddyy
Copy link
Contributor

ttddyy commented Feb 27, 2023

I faced an ordering issue for registering custom ObservationHandler beans to the observation registry.

The use case is that I want to add a logic that adds tags to all spans. In order to do it, I wrote a custom ObservationHandler implementation that adds key-values to an observation context in onStop(). Then, I’m relying on DefaultTracingObservationHandler to convert those key-values in observation to span tags.

When I define my custom handler bean, ObservationHandlerGrouping is responsible for registering such handler beans to the registry in order.

void apply(List<ObservationHandler<?>> handlers, ObservationConfig config) {
MultiValueMap<Class<? extends ObservationHandler>, ObservationHandler<?>> groupings = new LinkedMultiValueMap<>();
for (ObservationHandler<?> handler : handlers) {
Class<? extends ObservationHandler> category = findCategory(handler);
if (category != null) {
groupings.add(category, handler);
}
else {
config.observationHandler(handler);
}
}
for (Class<? extends ObservationHandler> category : this.categories) {
List<ObservationHandler<?>> handlerGroup = groupings.get(category);
if (!CollectionUtils.isEmpty(handlerGroup)) {
config.observationHandler(new FirstMatchingCompositeObservationHandler(handlerGroup));
}
}
}

The apply() method checks the category. If the handler is in a category, it is added to a handleGroup list. Those handlerGroups will be registered with FirstMatchingCompositeObservationHandler in the second for-loop. However, since my custom handler is not in a category, regardless I specify bean order or not, it is registered to the observation registry right away before the handlerGroups which contains DefaultTracingObservationHandler.

This ordering causes a problem.

The handler registration order becomes:

  1. my custom handler
  2. FirstMatchingCompositeObservationHandler which contains DefaultTracingObservationHandler.

When an observation starts, it goes through the handlers by the registered order, then on the stop, it goes by reverse order.

Since onStop is called in reverse order, DefaultTracingObservationHandler is called first and closes the current span. Then my custom handler tries to add key-values to the observation, but it will have no effect on the span since it was already closed.

A workaround is to use ObservationRegistryCustomizer bean to register my custom handler. It is because customizers are applied after observation handlers have registered.

I believe an observation handler that is not in any category needs to be registered after infrastructure handlers are registered.
Or, need a mechanism to choose whether to register a handler before/after handlerGroups registration.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 27, 2023
@philwebb philwebb added the theme: observability Issues related to observability label Feb 28, 2023
@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 7, 2023
@philwebb philwebb added this to the 3.2.x milestone Jun 7, 2023
@philwebb philwebb mentioned this issue Jun 7, 2023
31 tasks
@mhalbritter mhalbritter modified the milestones: 3.2.x, 3.2.0-M1 Jun 15, 2023
@mhalbritter
Copy link
Contributor

We now register uncategorized handlers after the categorized ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: observability Issues related to observability type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants