-
Notifications
You must be signed in to change notification settings - Fork 249
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
feat(telemetry)_: include device type in metrics #5669
Conversation
c64f5d3
to
348e9f2
Compare
✔️ status-go/prs/tests-rpc/PR-5669#1 🔹 ~2 min 51 sec 🔹 c64f5d3 🔹 📦 tests-rpc package |
Jenkins BuildsClick to see older builds (35)
|
8ec4005
to
c44f9fc
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.
My main objection is accessing telemetry client through WakuV2Service
, it's very confusing. The DeviceType
is taken from Messenger
, which also holds the TelemetryClient
.
Otherwise it's good, thank you!
c44f9fc
to
4c5f52e
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 the improvements 👍
711d6d7
to
74f9d5b
Compare
74f9d5b
to
a734ddf
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.
lgtm
Initializes telemetry client with device type and includes in all calls to telemetry server.
At the end of
injectAccountsIntoWakuService
, finds the installation based on installation ID set in messenger, and calls Waku service to set it in the telemetry client. Does nothing if the telemetry client is not set.Important changes:
Closes #