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

fix: add grpc service name to grpc metrics #352

Merged
merged 1 commit into from Jul 12, 2022
Merged

fix: add grpc service name to grpc metrics #352

merged 1 commit into from Jul 12, 2022

Conversation

pboros
Copy link
Contributor

@pboros pboros commented Jul 12, 2022

Load the environment from tags, add grpc_service to grpc request metric tags.

@codecov-commenter
Copy link

Codecov Report

Merging #352 (5a1125c) into main (328aaa8) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage   29.04%   29.04%           
=======================================
  Files          64       64           
  Lines        6721     6721           
=======================================
  Hits         1952     1952           
  Misses       4552     4552           
  Partials      217      217
Impacted Files Coverage Δ
server/metrics/metrics.go 100.00% <100.00%> (ø)
server/metrics/requests.go 88.57% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 328aaa8...5a1125c. Read the comment docs.

@pboros pboros marked this pull request as ready for review July 12, 2022 08:45
@pboros pboros force-pushed the pboros/add-grpc-service-name-to-metrics branch from 5a1125c to 2d08ce1 Compare July 12, 2022 13:55
rbarabas
rbarabas reviewed Jul 12, 2022
View reviewed changes
server/metrics/metrics.go
@@ -40,7 +40,7 @@ var (
func GetGlobalTags() map[string]string {
return map[string]string{
"service": config.DefaultConfig.Tags.Service,
"env": config.GetEnvironment(),
"env": config.DefaultConfig.Tags.Environment,
Copy link
Contributor

@rbarabas rbarabas Jul 12, 2022 •
edited

Choose a reason for hiding this comment

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

I'm actually retracting my stance here with https://github.com/tigrisdata/tigris/pull/349/files as I'm getting rid of the Tags structure in the Config entirely. Not a biggie as I can change it in PR #349 just an FYI.

@pboros pboros merged commit 065426c into main Jul 12, 2022
@pboros pboros deleted the pboros/add-grpc-service-name-to-metrics branch July 12, 2022 14:13
@tigrisdata-argocd-bot
Copy link
Collaborator

🎉 This PR is included in version 1.0.0-alpha.23 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@tigrisdata-argocd-bot
Copy link
Collaborator

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbarabas rbarabas rbarabas approved these changes

Assignees
No one assigned
Labels
released on @alpha released on @beta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pboros @codecov-commenter @tigrisdata-argocd-bot @rbarabas