Ability to customize the default ObservationEnvironmentRepositoryObservationConvention

[sergioasantiago]

Is your feature request related to a problem? Please describe.
Current implementation doesn't allow to customize the default ObservationEnvironmentRepositoryObservationConvention implementation. The default implementation adds to the lowCardinalityTags the application. This can be a problem when we have many applications, like our use case. This is exploding the cardinality for this metric when we expose this metric to prometheus and this is a known performance issue.

Describe the solution you'd like
I would like to be able to change the tags that are exported in metrics.

Describe alternatives you've considered
I tried to use an ObservationFilter as described in micrometer docs, but since it is a post-processing component it didn't take effect.

[ryanjbaxter]

Thanks for opening the issue @sergioasantiago.

I am curious on how you tried to add the ObservationFilter to the ObservationRegistry in your app, can you provide an example of what you tried?

[spring-cloud-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[sergioasantiago]

Hi 🧇

I apologize for the delay.

I ended up circumventing this by creating a GlobalObservationConvetion that will overwrite the one used by config-server.
This way I could overwrite getLowCardinalityKeyValues and getHighCardinalityKeyValues.
Bad thing is that it is a code duplication from here, since I can't override this bean.

import io.micrometer.common.KeyValues;
import io.micrometer.common.docs.KeyName;
import io.micrometer.observation.GlobalObservationConvention;
import io.micrometer.observation.Observation;
import org.jetbrains.annotations.NotNull;
import org.springframework.cloud.config.server.environment.ObservationEnvironmentRepositoryContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.util.StringUtils;

@Configuration
public class ObservationEnvironmentRepositoryGlobalObservationConvention
{
    enum LowCardinalityTags implements KeyName
    {

        /**
         * Implementation of the EnvironmentRepository.
         */
        ENVIRONMENT_CLASS
            {
                @NotNull
                @Override
                public String asString()
                {
                    return "spring.cloud.config.environment.class";
                }
            },

        /**
         * Application name for which properties are being queried for.
         */
        PROFILE
            {
                @NotNull
                @Override
                public String asString()
                {
                    return "spring.cloud.config.environment.profile";
                }
            },

        /**
         * Label for which properties are being queried for.
         */
        LABEL
            {
                @NotNull
                @Override
                public String asString()
                {
                    return "spring.cloud.config.environment.label";
                }
            },

        /**
         * Application name for which properties are being queried for.
         */
        APPLICATION
            {
                @NotNull
                @Override
                public String asString()
                {
                    return "spring.cloud.config.environment.application";
                }
            }
    }


    @Bean
    public GlobalObservationConvention<ObservationEnvironmentRepositoryContext> globalObservationConvention()
    {
        return new GlobalObservationConvention<>()
        {
            @NotNull
            @Override
            public KeyValues getLowCardinalityKeyValues(@NotNull ObservationEnvironmentRepositoryContext context)
            {
                KeyValues keyValues = KeyValues.empty();
                keyValues = appendIfPresent(keyValues, LowCardinalityTags.ENVIRONMENT_CLASS,
                    context.getEnvironmentRepositoryClass().getName());
                keyValues = appendIfPresent(keyValues, LowCardinalityTags.LABEL,
                    context.getLabel());
                return appendIfPresent(keyValues, LowCardinalityTags.PROFILE,
                    context.getProfile());
            }


            @NotNull
            @Override
            public KeyValues getHighCardinalityKeyValues(@NotNull ObservationEnvironmentRepositoryContext context)
            {
                KeyValues keyValues = KeyValues.empty();
                return appendIfPresent(keyValues, LowCardinalityTags.APPLICATION,
                    context.getApplication());
            }


            private KeyValues appendIfPresent(KeyValues keyValues, KeyName profile, String value)
            {
                if (StringUtils.hasText(value))
                {
                    keyValues = keyValues.and(profile.withValue(value));
                }
                return keyValues;
            }


            @Override
            public boolean supportsContext(@NotNull Observation.Context context)
            {
                return context instanceof ObservationEnvironmentRepositoryContext;
            }


            @Override
            public String getName()
            {
                return "spring.cloud.config.environment.find";
            }


            @Override
            public String getContextualName(@NotNull ObservationEnvironmentRepositoryContext context)
            {
                return "env find";
            }
        };
    }
}

[jonatan-ivanov]

Current implementation doesn't allow to customize the default ObservationEnvironmentRepositoryObservationConvention implementation.

It should, I think this might be a bug that should be fixed. If you look into the instrumentation:

spring-cloud-config/spring-cloud-config-server/src/main/java/org/springframework/cloud/config/server/environment/ObservationEnvironmentRepositoryWrapper.java

Lines 64 to 66 in 7adbc3f

	return DocumentedConfigObservation.ENVIRONMENT_REPOSITORY
	.observation(null, CONVENTION, () -> context, this.registry)
	.observe(() -> this.delegate.findOne(application, profile, label));

The first parameter of the observation method is the custom convention (hardcoded null), the second one is the default convention, see: ObservationDocumentation:

Observation observation(@Nullable ObservationConvention<T> customConvention, ObservationConvention<T> defaultConvention, ...)

We should figure out how to let the users inject a ObservationEnvironmentRepositoryObservationConvention instance into ObservationEnvironmentRepositoryWrapper (hopefully we can get away with an extra parameter and a bean creation on user-side).

The default implementation adds to the lowCardinalityTags the application. This can be a problem when we have many applications, like our use case.

I think I still agree with the instrumentation. The application still seems to be a low cardinality tag even if you have lots of them. Having a lot does not necessarily mean high cardinality, see this: https://develotters.com/posts/high-cardinality/
Please also notice that I'm talking about cardinality not your pain or costs. :)

You can do 3 things to mitigate this issue right now (4 after we add customization for the convention):

1. As you already found out you can use a global convention that you can configure in the registry.
2. You can use an ObservationFilter, I'm not sure I understand your comment about it being "a post-processing component it didn't take effect". It does take effect, other users are using it (it runs before onStop so dependin on your setup it might not take effect on metrics you create in onStart, by default this could be the case with LongTaskTimer).
3. You can use a MeterFilter, this might be the simplest, one-liner solution.