-
Notifications
You must be signed in to change notification settings - Fork 575
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
Make sure redis instrumentation follows semantic conventions #1627
Comments
@srikanthccv I'd like to help with these. I'd like to start here with redis, then move on to other if possible. |
Notional Plan:
|
Current attributes: "attributes": {
"db.statement": "GET ?",
"db.system": "redis",
"db.redis.database_index": 0,
"net.peer.name": "localhost",
"net.peer.port": 6379,
"net.transport": "ip_tcp",
"db.redis.args_length": 2
}, |
I created the tests, but found the following:
A couple issues that may be relavant (?): |
I assigned this to you. Please see open-telemetry/opentelemetry-python#3251 for updating the semconv. |
Thank you! Should this task (#1627) wait until we're aligned on semconv latest (which is currently 1.24.0)? Just brainstorming: Is it possible to add a layer in from opentelemetry.semconv import SemConv
semconv = SemConv(version="1.20.0");
semconv.span_attributes.SERVER_ADDRESS It doesn't completely solve the coupling, but may help break up the PRs. You could simply have one relatively small CR to make the new version available for use, then incrementally update usage in all of the sub-packages in separate PRs. This could allow skipping sub-package updates if say 1.21.0, 1.22.0, etc. come along before all the sub-packages are updated. Then follow up by deprecating unused versions as all the usage is updated. Edit: I'm just now realizing this library is versioned in lock-step, so nvm. |
@srikanthccv I created a PR for now it's pointing to the depended PR (the 1.20.0 change), but once that's merged I can open a real one on the main project. Is this approach acceptable? Basically I added tests and then fixed the impl where things weren't quite right. |
verifies the database semantic convention for redis (1.20.0) fixes minor issues with attributes to align with specification
No description provided.
The text was updated successfully, but these errors were encountered: