Opentelemetry Redis Instrumentation

[luneo7]

Adds Opentelemetry Redis Instrumentation, fixes #39011

Examples of how spans will be created:

[quarkus-bot]

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

[luneo7]

@brunobat used only the DbClientAttributesExtractor and not the ServerAttributesGetter or the NetworkAttributesGetter to set the peer address/port and such because the upstream CommandReporter is just sending a string and not an InetSocketAddress... so I would have to split the string with a colon pattern, and recreate the socket address object... so we are only using the db.connection_string to show the server, what do you think?

Some like:

...
      private static final Pattern COLON_PATTERN = Pattern.compile(":");
      private static final String SERVER_ADDRESS_KEY = "server.address";
      private static final String SERVER_PORT_KEY = "server.port";
      private static final Logger LOGGER = Logger.getLogger(CommandTrace.class);


       CommandTrace(final Map<String, String> attributes) {
            this.attributes = attributes;
            String peerAddress = peerAddress();
            if (peerAddress != null) {
                try {
                    String[] addressAndPort = COLON_PATTERN.split(peerAddress);
                    this.attributes.put(SERVER_ADDRESS_KEY, addressAndPort[0]);
                    if (addressAndPort.length > 1) {
                        this.attributes.put(SERVER_PORT_KEY, addressAndPort[1]);
                    }
                } catch (Exception e) {
                    LOGGER.debugf(e, "Failed to parse peerAddress %s", peerAddress);
                }
            }

        }
...

Also I've set the db name... which is kinda of right since the command reporter returns which DB is being used in the operation, just that in Redis they are numbers.... so the span name will be {operation} {dbName}.... I can also not set the DB name and the span name would be just {operation} since this is how the DbClientSpanNameExtractor works... what do you think about that span naming as well?

[luneo7]

Another thing that I noted is that the db.statement that the command reporter sends is actually the db.operation and we don't have a db.statement maybe later we can improve those by also tweaking the command reporter, and later changing our instrumentation... I think that what we have now is a good start though =]

[luneo7]

Hey @brunobat were you able to take a look at this?

[luneo7]

I've removed the db hash index from the span name and added it to an attribute... seems better =] so the span name will be only {operation}

[brunobat]

Hi @luneo7
Tried to run the tests but I got this error:

... no DOCKER_HOST environment variable ...

This shouldn't be needed.
I see that you started from the setup available for the redis-client IT. Please give it a shot with a more recent setup like the one in redis-devservices.

We will also need the documentation update.

[brunobat]

Thanks @luneo7 for the initiative.

[brunobat]

@Ladicek would you like to see something else in the spans for redis?

[Ladicek]

I don't feel qualified to review the code, but the set of attributes seems reasonable, given what we expose on the Vert.x Redis client side. I can add more there, just let me know, but I tried to copy what the Vert.x SQL client exposes.

[brunobat]

I don't feel qualified to review the code, but the set of attributes seems reasonable, given what we expose on the Vert.x Redis client side. I can add more there, just let me know, but I tried to copy what the Vert.x SQL client exposes.

Thanks @Ladicek, please don't feel pressured to review all the code. What I had in mind was exactly what you mentioned... Reviewing the tracked attributes to make sure we have the best set to show the users.

[luneo7]

@brunobat any new thoughts on this?

[brunobat]

@luneo7 Can you please add the client instrumentation the documentation list in here:
https://github.com/quarkusio/quarkus/blob/f76d812036f1e2ce3ca12d0bb5465e09bfa28212/docs/src/main/asciidoc/opentelemetry.adoc#L635-L634

I wonder if you have any project using Redis where you can try this out?

[brunobat]

Could you also please add a ON/OFF test for the feature, similar to this one?

quarkus/extensions/opentelemetry/deployment/src/test/java/io/quarkus/opentelemetry/deployment/instrumentation/GrpcOpenInstrumentationDisabledTest.java

Line 38 in 42ffdef

public class GrpcOpenInstrumentationDisabledTest {

[luneo7]

@brunobat I've created the issue vert-x3/vertx-redis-client#441 , added the test and updated the docs.

[luneo7]

@brunobat any other request?

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 7e14b56.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7e14b56.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

[brunobat]

Thanks very much @luneo7
It looks good to me.

[github-actions]

🙈 The PR is closed and the preview is expired.