-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
enhance productcatalogservice tracing support: add attributes to existing spans #143
Conversation
…ting spans Signed-off-by: Ziqi Zhao <[email protected]>
@cartersocha will you help review this? |
There was a problem hiding this 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?
Absolutely :) |
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 <[email protected]>
ping @puckpuck @julianocosta89 for review. I've added more attributes and events. Please help to review this, thanks :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🤩
@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. |
I think the merge automatically closed the issue. I just re-opened it. I agree it’s not done yet! 🧜🏻♂️ |
fix #50
I'm trying to fix
Changes
Adds attributes to existing span in GetProduct handler.