Implementation of SingleEntityExecution generates false-positives when used with opentelemetry instrumentation

[hanswesterbeek]

Hi,

Spring-data-jpa's use of Hibernate's getSingleResult often causes Hibernate to throw a NoResultException internally, which causes the otel-hibernate intrumentation module to flag the span as a failure, and recording the stacktrace in the Span and flagging it as 'error'.

This leads to false-positives especially when the Spring-data method returns an Optional, eg Optional<Foo> findByBar.

Clearly this issue exists at a crossroads between otel, spring-data and jpa. I think it is best reported here because I can kind of see that alternative Hibernate APIs like Optional<R> uniqueResultOptional() and R getSingleResultOrNull() exist on org.hibernate.query.SelectionQuery interface, which I hope can be used. That wouldn't set off the otel-instrumentation.

A fix doesn't seem trivial to me because jakarta.persistence.Query::getSingleResult doesn't have the right method and that is probably the interface spring-data has to talk to preserve compatibility with other jpa-providers. Happy to provide a PR if your team believes it possible and provide me with a few pointers.

[mp911de]

I think that the instrumentation is making invalid assumptions. One side is how an API is defined, the other one is how to react to it. One aspects that play into that is that JPA has defined to use exceptions to represent nullability.

Just because there is no result that doesn't mean that the data access has failed. Zooming out, I get the feeling that any open telemetry instrumentation happens on a best-effort basis leaving its users with a difficult experience instead of addressing issues more thoughtfully. That isn't the first time I see opentelemetry in action in such a way.

Looking into JPA providers, getSingleResult() is merely a wrapper around getResultList() enforcing API constraints. While we could have a wrapper that captures the non-unique case, I'm not sure that Spring Data is the right place to address this shortcoming. We're already compensating for a lot of JPA shortcomings. Compensating for otel shortcomings doesn't feel right and in fact, if someone decided to decorate JPA's Query they would suddenly see calls to list results in the context of single-result queries introducing an entire dimension of different problems.

[hanswesterbeek]

I see your point and it's the response I kind of expected as this issue is at a difficult cross-roads. Reason I asked it here is because I believed it to be the best hope for a solution.

I think the problem from the point of view of the Otel hibernate-instrumentation maintainers is that they can't distinguish between uses of getSingleResult() that should be flagged as an error and those that shouldn't be. They can't see the Optional wrapper-type which would hint at it.

I have zero hope of a resolution via the Hibernate project.

That said I understand if you guys would leave it here, fully appreciate what you say about JPA-shortcomings.

[mp911de]

I think the problem from the point of view of the Otel hibernate-instrumentation maintainers is that they can't distinguish between uses of getSingleResult() that should be flagged as an error and those that shouldn't be.

That is exactly my point. They are integrated however in ways that would allow them to recognize surroundings through their agent and that could help to react within the context.

Taking a step back: Why do you even run spans at the Hibernate entry point and not the repository?

I have zero hope of a resolution via the Hibernate project.

That isn't the right place either.

I consider creating Spans on Hibernate interaction a bit fishy. If you really want to stick to that pattern, than at least NonUniqueException and NoResultException should be merely indicators for the result arity and not fail the span.

[hanswesterbeek]

It's not me explicitly creating spans around hibernate interactions, it's the otel jvm-agent, which has the hibernate instrumentation on board that automatically instruments the JPA. It works pretty well, this is the only problem we've had with it, so not bad at all.

The jvm-agent also contains spring-data instrumentation, which works fine.

For now we have indeed disabled the hibernate instrumentation altogether using OTEL_INSTRUMENTATION_HIBERNATE_ENABLED=false. That's more coarse-grained than we would like but it's ok.

Thanks for your time.