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.

Sign up for GitHub

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

Jump to bottom

Generate RPC metrics from YAML #93

Merged
merged 6 commits into from
Aug 2, 2023
Merged

Generate RPC metrics from YAML #93

merged 6 commits into from
Aug 2, 2023

Conversation

joaopgrassi
Copy link
Member

@joaopgrassi joaopgrassi commented Jun 7, 2023 •
edited
Loading

Address part of #74

CC @jsuereth @trask @lmolkova @jack-berg as previous authors

@joaopgrassi joaopgrassi requested review from a team June 7, 2023 12:12
@joaopgrassi joaopgrassi mentioned this pull request Jun 29, 2023
Merged
@joaopgrassi joaopgrassi force-pushed the feat/rpc-metrics-yaml branch from c5a555a to 4b3a879 Compare July 6, 2023 08:41
@joaopgrassi joaopgrassi force-pushed the feat/rpc-metrics-yaml branch from 4b3a879 to 89c4961 Compare July 7, 2023 07:49
jsuereth
jsuereth approved these changes Jul 27, 2023
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Approving this as an incremental fix, but I think we should add the appropriate attributes to metrics to be explicit. I also think we may want to re-examine some as the language makes it confusing - I don't think we intended to share as many attributes as are shared.

@joaopgrassi
Copy link
Member Author

joaopgrassi commented Jul 29, 2023 •
edited
Loading

I will work on solving the conflicts and make sure the issue isn't closed by this PR.

This was referenced Jul 31, 2023
Closed
Open
@joaopgrassi
Copy link
Member Author

@jsuereth I updated the PR, solved the conflicts. This should be good to go.

To address your comments, I updated the issue #74 adding the left-over to generate the attributes for all metrics as a follow up. The ticket won't be closed as I changed the description in the PR (I hope!)

For the other comment about re-examine the wording/attributes, I created another issue #219

jsuereth
jsuereth approved these changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up! LGTM for the incremental progress.

@jsuereth jsuereth merged commit b89a033 into open-telemetry:main Aug 2, 2023
@joaopgrassi joaopgrassi deleted the feat/rpc-metrics-yaml branch August 7, 2023 15:22
bryce-b pushed a commit to bryce-b/semantic-conventions that referenced this pull request Aug 8, 2023
@joaopgrassi @bryce-b
Generate RPC metrics from YAML (open-telemetry#93)
3097776
rapphil pushed a commit to rapphil/semantic-conventions that referenced this pull request Aug 14, 2023
@joaopgrassi @rapphil
Generate RPC metrics from YAML (open-telemetry#93)
a491f82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexanderWert AlexanderWert AlexanderWert approved these changes

@jsuereth jsuereth jsuereth approved these changes

Assignees

@reyang reyang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@joaopgrassi @jsuereth @AlexanderWert @reyang