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

Enable optional trust domain label for all metrics #5652

Open
graceajibade opened this issue Nov 19, 2024 · 3 comments · May be fixed by #5673
Open

Enable optional trust domain label for all metrics #5652

graceajibade opened this issue Nov 19, 2024 · 3 comments · May be fixed by #5673
Labels
priority/backlog Issue is approved and in the backlog

Comments

@graceajibade
Copy link

graceajibade commented Nov 19, 2024 •
edited
Loading

From the telemetry doc, there are some labels that accompany metrics, we would find it useful in our deployment to be able to set a custom label to distinguish different sets of spire-servers from each other.

To this end, we could enable the use of ServiceName and EnableServiceLabel (which is from go-metric config) in metrics.go, which would give the ability to optionally pass a ServiceName and enable that ServiceName to be turned to a label. Looking at go-metric's implementation, it looks like it always uses "service" as the label. It would be nice if we could get it to be some kind of custom label, for example support for global labels could be introduced in metrics.go.

I would be happy to implement this if it gets accepted.
@graceajibade graceajibade changed the title Enable to use of ServiceName and EnableServiceLabel for telemetry Enable the use of ServiceName and EnableServiceLabel for telemetry Nov 19, 2024
@amartinezfayo amartinezfayo added the triage/in-progress Issue triage is in progress label Nov 19, 2024
@sorindumitru
Copy link
Contributor

Some more details, this is to help with having multiple trust domains publish metrics to the same destination/namespace. Having the trust domain as a label makes it easier to use, rather than including it in the metric namespace.

I don't think anything else apart from the trust domain id/name is needed.

@rturner3
Copy link
Collaborator

That sounds ok to me. I think we would just want to make this an optional label that is disabled by default and can be enabled via a config property in the telemetry config section to avoid unintentionally increasing the volume of telemetry data emitted for existing larger deployments.

@graceajibade graceajibade linked a pull request Nov 28, 2024 that will close this issue
Open
@graceajibade
Copy link
Author

That sounds ok to me. I think we would just want to make this an optional label that is disabled by default and can be enabled via a config property in the telemetry config section to avoid unintentionally increasing the volume of telemetry data emitted for existing larger deployments.

Making it an optional label works well. I will edit the title of this issue to reflect that.

@graceajibade graceajibade changed the title Enable the use of ServiceName and EnableServiceLabel for telemetry Enable optional trust domain label for all metrics Nov 28, 2024
@amartinezfayo amartinezfayo added priority/backlog Issue is approved and in the backlog and removed triage/in-progress Issue triage is in progress labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority/backlog Issue is approved and in the backlog
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Enable optional trust domain label for all metrics graceajibade/spire
4 participants
@sorindumitru @rturner3 @amartinezfayo @graceajibade