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

enhance productcatalogservice tracing support: add attributes to existing spans #143

Merged

Conversation

fatsheep9146
Copy link
Contributor

fix #50

I'm trying to fix

  • Services extend automatic instrumentation.
    • New attributes/events attached to existing spans.

Changes

Adds attributes to existing span in GetProduct handler.

…ting spans

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
@fatsheep9146 fatsheep9146 requested a review from a team June 16, 2022 09:45
@fatsheep9146
Copy link
Contributor Author

The screen shot is like following

image

@fatsheep9146
Copy link
Contributor Author

@cartersocha will you help review this?

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Hello @fatsheep9146, could you add some extra attributes?
And also some events?

@fatsheep9146
Copy link
Contributor Author

Hello @fatsheep9146, could you add some extra attributes? And also some events?

Absolutely :)

@puckpuck
Copy link
Contributor

I'd like to see the ListProducts and SearchProducts handlers with additional attributes as well. We can add the total products returned to both, and the search query to the latter.

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
@fatsheep9146
Copy link
Contributor Author

ping @puckpuck @julianocosta89 for review. I've added more attributes and events. Please help to review this, thanks :)

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

LGTM! 🤩

@cartersocha
Copy link
Contributor

@fatsheep9146 please update the issue to reflect your changes.

Merging now 🦑

@cartersocha cartersocha merged commit 6620aa3 into open-telemetry:main Jun 19, 2022
@fatsheep9146
Copy link
Contributor Author

@fatsheep9146 please update the issue to reflect your changes.

Merging now 🦑

@cartersocha I saw that you closed the issue #50. I think the issue is not ready to be closed, there are still several features needed to be resolved.

@cartersocha
Copy link
Contributor

I think the merge automatically closed the issue. I just re-opened it. I agree it’s not done yet! 🧜🏻‍♂️

GaryPWhite pushed a commit to wayfair-contribs/opentelemetry-demo that referenced this pull request Jun 30, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
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.

Review and enhance tracing support for product catalog service (Go)
4 participants