Explicitly set OTel Service Instance ID#700
Explicitly set OTel Service Instance ID#700mhutchinson merged 2 commits intotransparency-dev:mainfrom
Conversation
This uses os.Hostname, falling back on a UUID if for some reason it can't be looked up. This should work on VMs and CloudRun instances. Fixes transparency-dev#699.
| } | ||
| } | ||
|
|
||
| instanceID, err := os.Hostname() |
There was a problem hiding this comment.
I think this returns the instanceName, right? In which case s/instanceID/instanceName.
There was a problem hiding this comment.
It is intended to be used as an Instance ID though when we set it in OTel. As in, we ultimately call semconv.ServiceInstanceIDKey.String(instanceID).
cmd/tesseract/gcp/otel.go
Outdated
| } | ||
| resources, err := resource.New(ctx, | ||
| resource.WithTelemetrySDK(), | ||
| resource.WithFromEnv(), // unpacks OTEL_RESOURCE_ATTRIBUTES |
There was a problem hiding this comment.
What takes precedence, WithFromEnv or resource.WithAttributes? Today, we can set the InstanceID with https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/, and we tell folks to do so in the codelab. Will it still work after this PR?
There was a problem hiding this comment.
@mhutchinson I think your commit and my PR crossed each other: I like your initial implementation without the 8 random characters better. If we can, let's reconcile this with a solution that ensures folks going through the codelab don't face log entries with errors and broken monitoring straight away?
There was a problem hiding this comment.
Backed this out, and swapped the order of initializing the environment variables and code configured values. This addresses your comment, and ultimately gives the user more flexibility and control.
Whichever is last wins. Previously we used the env variables as the base, and then the binary would replace them. This way around is more user-first: if the user is strongly opinionated that in their telemetry they want, e.g. service name to be something different, they can do.
mhutchinson
force-pushed
the
b699-hostname
branch
from
02dfef4 to
02a3452
Compare
2 participants
This uses os.Hostname, falling back on a UUID if for some reason it can't be looked up. This should work on VMs and CloudRun instances.
Fixes #699.