-
Notifications
You must be signed in to change notification settings - Fork 904
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
Add support for spring-pulsar 1.0 #13320
base: main
Are you sure you want to change the base?
Add support for spring-pulsar 1.0 #13320
Conversation
@jaydeluca Excuse me, could you kindly review this when you have some time? Thanks! |
} | ||
|
||
dependencies { | ||
library("org.springframework.pulsar:spring-pulsar:1.2.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module is named 1.0
but tests 1.2.0
pass { | ||
group.set("org.springframework.pulsar") | ||
module.set("spring-pulsar") | ||
versions.set("[1.2.0,]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you also add assertInverse.set(true)
This verifies that instrumentation does not apply to earlier versions.
trace -> { | ||
trace.hasSpansSatisfyingExactly( | ||
span -> | ||
span.hasName(String.format("%s receive", OTEL_TOPIC)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't make sense to have receive
in a separate trace. Have a look at spring-kafka instrumentation. It faces similar challenges as the pulsar instrumentation you are working on. The shape of the telemetry changes depending on the otel.instrumentation.messaging.experimental.receive-telemetry.enabled
flag (defaults to false). When it is false receive
then spring-pulsar instrumentation should suppress the receive
span created by the pulsar
instrumentation. When it is true then process
span should be child of the receive
span and process
span should have a span link to the producer
span. Implementing this may be a bit complicated, let us know if you aren't able to figure this out.
import org.testcontainers.containers.PulsarContainer; | ||
import org.testcontainers.utility.DockerImageName; | ||
|
||
public class SpringPulsarTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class SpringPulsarTest { | |
class SpringPulsarTest { |
|
||
@Test | ||
@SuppressWarnings("deprecation") // using deprecated semconv | ||
public void shouldCreateSpansForMessageProcess() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public void shouldCreateSpansForMessageProcess() { | |
void shouldCreateSpansForMessageProcess() { |
Resolves #13309