-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Add user agent metrics #492
Conversation
- Add metric about yorkie-sdk-type/version, projectID/Name, server instance name etc. - every grpc request success, collect metric about yorkie-sdk information
Codecov Report
@@ Coverage Diff @@
## main #492 +/- ##
==========================================
- Coverage 48.68% 48.33% -0.35%
==========================================
Files 69 69
Lines 5953 5896 -57
==========================================
- Hits 2898 2850 -48
+ Misses 2714 2705 -9
Partials 341 341
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
294f235
to
23732c5
Compare
- 1. Move collecting user-agent metrics to interceptors - 2. Remove gRPC dependency from metrics - 3. Extract header constants
23732c5
to
ad2deb6
Compare
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.
Thanks for your contribution.
I think it would be good to continue improving this by deploying it and watching actual data.
return nil, fmt.Errorf("get hostname: %w", err) | ||
hostname := conf.Hostname | ||
if hostname == "" { | ||
hostname, err := os.Hostname() |
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.
This hostname
value inside if
statement shadows hostname
in line 68.
Therefore hostname
will not be updated.
I'll fix this in #521
What this PR does / why we need it:
yorkie-js-sdk
,yorkie-ios-sdk
,yorkie-android-sdk
need to set header and put yorkie-sdk informationshostname
when the server start (hostname is used in metrics etc.)$ yorkie server ... --hostname {host name value} ...
Backend.Hostname
field valueWhich issue(s) this PR fixes:
Fixes #393
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation:
Checklist: