-
Notifications
You must be signed in to change notification settings - Fork 97
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: add telemetry #1981
feat: add telemetry #1981
Conversation
|
21ac914
to
f8f728a
Compare
|
f8f85f1
to
e37f69d
Compare
f21a1e1
to
ef2117c
Compare
Preview of ubuntu-focal/go1.21 tests in 1f69272🔍 View Details on Terramate Cloud .
cmd/terramate/cli
cmd/terramate/cli/cliconfig
cmd/terramate/cli/telemetry
generate
generate/genfile
generate/genhcl
hcl
|
Preview of macos-ventura/go1.21 tests in 1f69272🔍 View Details on Terramate Cloud .
cmd/terramate/cli
cmd/terramate/cli/cliconfig
cmd/terramate/cli/telemetry
generate
generate/genfile
generate/genhcl
hcl
|
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 but please address the comments in the public types and values.
// Copyright 2023 Terramate GmbH | ||
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
//go:build localhostEndpoints |
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.
Why don't you use environment variables?
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.
I'm using the same approach as the checkpoint API here. It could be more convenient for debugging, but beyond that there is no reason to make the endpoint address configurable, so I'd just go with the existing scheme.
a1dbd15
to
fca5a8d
Compare
fca5a8d
to
859a1d7
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
Well done!!
🚀
859a1d7
to
1f69272
Compare
What this PR does / why we need it:
This PR adds telemetry to collect metrics about which commands are used most actively. We already have some general insights from the checkpoint API, but this gives more detailed data.
Documentation will be updated accordingly.
Special notes for your reviewer:
Does this PR introduce a user-facing change?