Skip to content
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 Vert.x 4.x #3143

Closed
sirinn opened this issue Oct 13, 2021 · 15 comments
Closed

Add support for Vert.x 4.x #3143

sirinn opened this issue Oct 13, 2021 · 15 comments
Labels
inst: others All other instrumentations inst: vertx Eclipse Vert.x instrumentation type: enhancement type: feature request
Milestone

Comments

@sirinn
Copy link

sirinn commented Oct 13, 2021

Hi,

according to your docs https://docs.datadoghq.com/tracing/setup_overview/compatibility_requirements/java/. There is no support for Vert.x 4.x upgrade. It has implemented many new tracing functionality. It would be appreciated if you can continue with the support for next major release.

Thanks

@devinsba
Copy link
Contributor

@sirinn thank you for the input, to help us scope the effort could you tell us what vertx modules you are using? IE: for vertx 3 we support web, rx, redis, and mysql.

Also if you could point us to (or create) a relatively simple sample application that would help us make sure we support the patterns that you are using.

@devinsba devinsba added type: enhancement type: feature request inst: others All other instrumentations inst: vertx Eclipse Vert.x instrumentation labels Oct 13, 2021
@sirinn
Copy link
Author

sirinn commented Oct 14, 2021

Hi @devinsba,

I would expect support for the core and web module at minimum. Redis would be nice as well. I can prepare some examples if you can give me pointers where are such examples for vertx3.

Another question is the support for eventbus on vertx. They have added support for much more advanced tracing, is this also something that can be done ? I'm not entirely sure show does this integrate into DD exactly. Maybe you can help me fill in the blanks. https://vertx.io/docs/vertx-opentelemetry/java/#_eventbus_tracing.

@sirinn
Copy link
Author

sirinn commented Oct 25, 2021

Hey, @devinsba

I had a quick look, and I can see so far that the problem is that class that is instrumented no longer exists. io.vertx.core.http.impl.HttpServerResponseImpl has been replaced by two new ones -> io.vertx.core.http.impl.Http1xServerResponse and io.vertx.core.http.impl.Http2xServerResponse. There has been no big change in the way how we use the http server or anything else. Therefore I don't see that examples would bring something new to the table.

I don't see any other changes in VertxDecorator or any other helper classes. The trace begin instrumentation hasn't changed. And this correlates with what we can see currently. (Spans not being closed properly and mixing of different request in one trace).

Can you please let me know if there is some ETA you could deliver this update ?

@bergbrains
Copy link

We are also in need of Vertx 4.x support.

@chris-brace
Copy link

chris-brace commented Nov 2, 2021

@devinsba

could you tell us what vertx modules you are using? IE: for vertx 3 we support web, rx, redis, and mysql.

super important for us as well. we're using web, redis, postgres here.

@devinsba
Copy link
Contributor

I don't know when we will end up being able to upgrade to vert.x 4, to get something working you can use the Opentracing support that Vertx provides: https://vertx.io/docs/vertx-opentracing/java/

Then if you add implementation group: 'io.opentracing', name: 'opentracing-util', version: '0.33.0' you can add the datadog tracer as the tracer: with a modified version of the second example, with the datadog javaagent GlobalTracer will return an OpenTracing tracer

Tracer tracer = GlobalTracer.get();
Vertx vertx = Vertx.vertx(new VertxOptions()
  .setTracingOptions(
    new OpenTracingOptions(tracer)
  )
);

@sirinn
Copy link
Author

sirinn commented Nov 11, 2021

Hey @devinsba,

thanks for reaching back, we tried this workaround even before writing here and our major problem with it is that the DD agent is still trying to execute all the broken instrumentation even on unsupported version of vert.x 4 and is creating a lot of noise and duplication of traces. Where it's really 50/50 chance of you finding the broken trace from instrumentation or correct one from custom tracing using vert.x open tracing extension. It really became unusable for us right now, which is very unfortunate as vert.x 4 major version was released some time ago.

Is there something we can do to help you give this integration a priority ? Official upgrade request from big tech customer ?

Would there be a possibility to disable the instrumentation and only allow custom tracing on DD agent ? Deploying a dedicated OpenTracing collector is not an option for us as we are also using the DD agent for CPU profiling.

And one more question, could you please confirm that you would have time to review/approve/release a public contribution for vert.x 4 support if it was contributed by someone ?

@gsaqui
Copy link

gsaqui commented Dec 22, 2021

@sirinn did you ever receive an answer from @devinsba via a different communication channel? Just wondering if there has been any movement around this issue as it's impacting us as well.

@sirinn
Copy link
Author

sirinn commented Dec 22, 2021

Hey @gsaqui,

No, I haven't received any further answers or communications from @devinsba or anyone else from this project. We are also not satisfied with the insufficient movement on this front and started considering switch to different tracing solutions, if no workaround or solution is found soon.

@devinsba
Copy link
Contributor

Sorry folks for the delay, to answer a couple questions:

Yes you can disable automatic instrumentation using system properties, dd.trace.integration.<name>.enabled=false. So for vert.x I would expect probably wanting to disable netty, and vertx. There are potentially others but if you have tracer debug logging enabled (dd.trace.debug=true) you can see what gets activated and disable them if needed. (also note properties can be converted to environment variables by converting to uppercase and swapping all . and - with underscores)

Second, we definitely do accept community contributions. Review of something like this would be a little easier than something completely brand now since we can compare to the old version. See #3210 and #3211 for recent examples

Finally, the fact that it is partially activating for Vertx 4 would seem to be a bug, would you be able to reach out to support@datadoghq.com to share debug logs so we can figure that out, even with support for vertx 4 specifically added we would need to fix whatever you are seeing

@gsaqui
Copy link

gsaqui commented Apr 2, 2022

Hi @devinsba - is there any plan for Vertx 4.0 to be supported?

@LuboViluda
Copy link
Contributor

Hello @devinsba,

  • I have opened Vertx 4 issue with DD support for our account.
  • I could confirm that we were able to disable intergration by env vars as you described, ofc then everything needs to be instrumented manually.
  • I have opened MR with changes to migrate instrumentation to Vert.x 4. I'm not sure about test suite changes needed (I tried to migrate from Vert.x 3 web suite) - more info in MR - 3143 add support for vertx web 4 #3557. Could you take a look and provide some guidance on how to proceed? MR
  • kudos to @sirinn for describing some of the migration steps

Regards,
Lubo

@akleiman
Copy link

With the MR above merged, is this issue resolved?

@akshaypatidar1999
Copy link
Contributor

akshaypatidar1999 commented Aug 14, 2023

@devinsba Is there any plan to integrate redis, and MySQL modules for vertx4?
We are currently upgrading our services to vertx 4 but are not able to get spans for redis/mysql queries

I will be happy to contribute to these enhancements

@devinsba
Copy link
Contributor

Please reach out to support@datadoghq.com with that feature request

@devinsba devinsba added this to the 0.103.0 milestone Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inst: others All other instrumentations inst: vertx Eclipse Vert.x instrumentation type: enhancement type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants