r2dbc / r2dbc-proxy

R2DBC Proxying Framework
https://r2dbc.io
Apache License 2.0
141 stars 21 forks source link

ObservationProxyExecutionListener + virtual threads = 💥 #149

Open cfredri4 opened 1 week ago

cfredri4 commented 1 week ago

Thread name is a low cardinality key but with virtual threads there are millions of threads which leads to e.g. prometheus going 💥.

ttddyy commented 6 days ago

Thanks for the report.

To work around this issue, you can create a custom convention that does not record the thread name, or use an observation filter to remove the thread name tag.

Here’s an example of how you can create a custom convention:

public interface SimpleQueryObservationConvention extends QueryObservationConvention {

    @Override
    default KeyValues getLowCardinalityKeyValues(QueryContext context) {
        Set<KeyValue> keyValues = new HashSet<>();
        keyValues.add(KeyValue.of(R2dbcObservationDocumentation.LowCardinalityKeys.CONNECTION, context.getConnectionName()));
        return KeyValues.of(keyValues);
    }

}

And here’s an example of an observation filter:

public class MyObservationFilter implements ObservationFilter {

    @Override
    public Observation.Context map(Observation.Context context) {
        if (context instanceof QueryContext) {
            context.remove(R2dbcObservationDocumentation.LowCardinalityKeys.THREAD.asString());
        }
        return context;
    }
}

I will likely add this custom convention implementation to the library, allowing users to choose between the default or this custom convention.

If you are using Spring Boot, defining a convention or observation filter bean should be automatically picked up, if I remember correctly.

cfredri4 commented 5 days ago

Thank you, this is what I'm doing now. But I opened the issue as I believe the behavior should be reversed; the safest behavior that will always work would be for thread name to not be included, and then it instead can be added as an option if you know you have a low number of threads.