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

Tracing support #625

Merged
merged 7 commits into from
May 24, 2020
Merged

Tracing support #625

merged 7 commits into from
May 24, 2020

Conversation

vietj
Copy link
Member

@vietj vietj commented May 13, 2020

Vert.x 4 supports tracing.

Fixes #653

@vietj vietj mentioned this pull request May 13, 2020
84 tasks
@vietj
Copy link
Member Author

vietj commented May 22, 2020

Pushed a new approach wiring the tracing in the client layer instead of the connection layer as the tracing cares about the client context before all.

@vietj vietj marked this pull request as ready for review May 22, 2020 10:19
@vietj vietj changed the title Tracing WIP Tracing support May 22, 2020
Copy link
Member

@BillyYccc BillyYccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're also missing db.url tag, it could be added if we support exporting sqlConnectOptions to a connection url.

@vietj
Copy link
Member Author

vietj commented May 22, 2020

db.url tag is not defined by opentracing and it is actually a Java-ish thing inherited from JDBC so I'm not sure we need that for the moment, e.g you don't have such tag if you have a JavaScript microservice

@BillyYccc
Copy link
Member

db.url is required in OpenTelemetry spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/database.md

It's not hard to support this after #644,
we could get SqlConnectOptions transformed into a short url like [dbkind]://[username]@[host]:[port]/[databasename]

@vietj
Copy link
Member Author

vietj commented May 23, 2020

please don't name that kind :-)

@vietj
Copy link
Member Author

vietj commented May 23, 2020

kind is typically the name you use when you can't find a good name :-)

@BillyYccc
Copy link
Member

I couldn't find an appropriate name for that either so I use it to show the usage 😅

Copy link
Member

@BillyYccc BillyYccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks very good as the first version after we settle the minor changes.

@vietj
Copy link
Member Author

vietj commented May 23, 2020

@BillyYccc as for naming and tags, I believe we need to have in master a definition of the tags we are going to use commonly in this project so each tracing integration can do a mapping of such tag to its own tagging system.

There are chances we will use the opentracing and/or opentelemetry list and then figure out. And then we will revisit this project for naming if required.

@vietj vietj added this to the 4.0.0 milestone May 23, 2020
@vietj
Copy link
Member Author

vietj commented May 24, 2020

I think the right name is dbms or rdbms

@vietj vietj merged commit 05ed43b into master May 24, 2020
@vietj vietj deleted the tracing branch July 2, 2020 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracing support
2 participants