-
Notifications
You must be signed in to change notification settings - Fork 174
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
Is db.connection_string
necessary?
#724
Comments
@open-telemetry/java-instrumentation-approvers do you know of anything important being done with |
Looking into java instrumentations, it seems database instrumentation either don't set connection string or set host and port are covered with No usages in python-contrib JS-contrib uses So it seems individual components of connection string are in most cases reported through other attributes and the only part of connection string that is not represented by any existing ones is the driver name. |
Maybe we should ping other maintainers here to get a final opinion? |
cc @open-telemetry/go-approvers @open-telemetry/javascript-approvers @open-telemetry/python-approvers @open-telemetry/opentelemetry-python-contrib-approvers @open-telemetry/dotnet-approvers @open-telemetry/dotnet-contrib-approvers |
Connection strings can have passwords. Example from Go: https://github.com/go-sql-driver/mysql/?tab=readme-ov-file#dsn-data-source-name Example from .NET: https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/connection-string-syntax Sometimes the libraries have connection string parsing function that could allow redacting. I think it should be at least opt-in. However, I rather support removal and welcome other attributes like
People should use e.g. logs for that. These attributes should help in relating spans/metrics to concrete databases each to find e.g. slow databases. |
We think that changing the default behavior to omit/exclude the |
@breedx-splk can you provide a bit more details about how you are using |
also, is this only for Java customers, or includes other languages as well? |
I'd also prefer to drop As mentioned above, in js-contrib we re-construct connection strings from other data that's added to the span already (host, port, db name). I just reviewed all of our usages, and I think 100% of the data we add via the |
@breedx-splk is there some specific information that you get from the connection string that's not available via other attributes? |
Sure, yeah. From what I understand, our APM UI leverages the |
@breedx-splk we can add a flag in Java instrumentation to keep around |
Thanks, I appreciate that offer @trask. I'm checking with our internal folks to see what they think. Another option too is to make it configurable, default it to false (excluded), and allow a distro to override the default (that way we can manage the time we need to keep it around while vanilla can remain spec compliant). |
Connection strings can have secrets and have very specific format which makes them not that useful for visualization tooling (or user queries).
They can also be long and redundant on every span.
They can (secrets redacted) be useful to log to help investigate configuration issues, but probably should not appear on spans/metrics.
Proposal is to remove the attribute or make it opt-in.
The text was updated successfully, but these errors were encountered: