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

Make sure database instrumentations follow semantic conventions #159

Closed
lzchen opened this issue Oct 28, 2020 · 9 comments
Closed

Make sure database instrumentations follow semantic conventions #159

lzchen opened this issue Oct 28, 2020 · 9 comments
Labels

Comments

@lzchen
Copy link
Contributor

lzchen commented Oct 28, 2020

From specs.

@srikanthccv
Copy link
Member

Hi @lzchen, I would like to work on this.

I have read the specification and gone through some database instrumentations. Here are the thins I noticed.

  1. Spec has db.system for DBMS identifier but the instrumentations have db.type.
  2. Similar to 1 as per spec db.name is used to report the name of the database being accessed but db.instance is created for instrumentations.
  3. Redis doesn't seem to have any one of [net.peer.name, net.peer.ip] .

@lzchen lzchen transferred this issue from open-telemetry/opentelemetry-python Nov 9, 2020
@srikanthccv
Copy link
Member

srikanthccv commented Nov 9, 2020

Hi @lzchen , I tried to address this here.

@toumorokoshi left some comments I think there's still some reconciliation that needs to happen on the specification and the fields emitted, especially on the MongoDB side.

Some db specific call level attributes are added which are not mentioned in spec. What are your thoughts on this?

@toumorokoshi
Copy link
Member

Hey @lonewolf3739! Sorry about the hassle, but would you mind sending your PR again? I think the libraries moved to this repo while this PR was still in discussion.

Regarding:

I think there's still some reconciliation that needs to happen on the specification and the fields emitted, especially on the MongoDB side.

Here's a spec ticket to start a discussion. I think some of this may be follow-up work as well, so the PR as-is would be great to merge in.

@srikanthccv
Copy link
Member

@toumorokoshi I don't mind opening PR here again. I just wanted to see if others can chime in and share their opinions.

@lzchen
Copy link
Contributor Author

lzchen commented Nov 11, 2020

@lonewolf3739

I don't mind opening PR here again. I just wanted to see if others can chime in and share their opinions.

The extra attributes not defined in the specs can be left until the specs are finalized.

@github-actions
Copy link

github-actions bot commented Apr 6, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@awssandra
Copy link

Are there any more remaining?

@github-actions github-actions bot removed the backlog label May 29, 2021
@srikanthccv srikanthccv removed their assignment May 30, 2021
@srikanthccv
Copy link
Member

srikanthccv commented May 30, 2021

@awssandra We haven't made any changes since last time. I think there would be some gaps between the specification and current implementation.

@lzchen lzchen added the triaged label Jun 2, 2021
@srikanthccv
Copy link
Member

I will create a new issue with the same title and epic containing all the db instrumentations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants