Skip to content

Commit

Permalink
Cleanup UID/InstanceID duplicity. Also remove override_instance_id
Browse files Browse the repository at this point in the history
…config option (#1125)

* Cleanup UID/InstanceID duplicity

* rm override_instance_id from documentation
  • Loading branch information
mariomac authored Sep 3, 2024
1 parent fe444df commit ecc9dc1
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 61 deletions.
8 changes: 0 additions & 8 deletions docs/sources/configure/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -604,14 +604,6 @@ instead of trying to automatically resolve the host name.

This option takes precedence over `dns`.

| YAML | Environment variable | Type | Default |
| ---------------------- | ------------------- | ------ | ------- |
| `override_instance_id` | `BEYLA_INSTANCE_ID` | string | (unset) |

If set, Beyla will use this value directly as instance ID of any instrumented
process. If you are managing multiple processes from a single Beyla instance,
all the processes will have the same instance ID.

### Kubernetes decorator

If you run Beyla in a Kubernetes environment, you can configure it to decorate the traces
Expand Down
2 changes: 1 addition & 1 deletion pkg/export/otel/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ var DefaultBuckets = Buckets{

func getAppResourceAttrs(hostID string, service *svc.ID) []attribute.KeyValue {
return append(getResourceAttrs(hostID, service),
semconv.ServiceInstanceID(service.Instance),
semconv.ServiceInstanceID(string(service.UID)),
)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/export/otel/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ func otelHistogramConfig(metricName string, buckets []float64, useExponentialHis
func (mr *MetricsReporter) metricResourceAttributes(service *svc.ID) attribute.Set {
attrs := []attribute.KeyValue{
request.ServiceMetric(service.Name),
semconv.ServiceInstanceID(service.Instance),
semconv.ServiceInstanceID(string(service.UID)),
semconv.ServiceNamespace(service.Namespace),
semconv.TelemetrySDKLanguageKey.String(service.SDKLanguage.String()),
semconv.TelemetrySDKNameKey.String("beyla"),
Expand Down
4 changes: 2 additions & 2 deletions pkg/export/prom/prom.go
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ func (r *metricsReporter) labelValuesSpans(span *request.Span) []string {
span.TraceName(),
strconv.Itoa(int(request.SpanStatusCode(span))),
span.ServiceGraphKind(),
span.ServiceID.Instance,
string(span.ServiceID.UID), // app instance ID
job,
"beyla",
}
Expand All @@ -737,7 +737,7 @@ func (r *metricsReporter) labelValuesTargetInfo(service svc.ID) []string {
service.HostName,
service.Name,
service.Namespace,
service.Instance,
string(service.UID), // app instance ID
job,
service.SDKLanguage.String(),
"beyla",
Expand Down
2 changes: 0 additions & 2 deletions pkg/internal/request/span_getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ func SpanOTELGetters(name attr.Name) (attributes.Getter[*Span, attribute.KeyValu
}
case attr.Service:
getter = func(s *Span) attribute.KeyValue { return ServiceMetric(s.ServiceID.Name) }
case attr.ServiceInstanceID:
getter = func(s *Span) attribute.KeyValue { return semconv.ServiceInstanceID(s.ServiceID.Instance) }
case attr.ServiceName:
getter = func(s *Span) attribute.KeyValue { return semconv.ServiceName(s.ServiceID.Name) }
case attr.ServiceNamespace:
Expand Down
7 changes: 2 additions & 5 deletions pkg/internal/svc/svc.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ const (
// ID stores the metadata attributes of a service/resource
// TODO: rename to svc.Attributes
type ID struct {
// UID might coincide with other fields (usually, Instance), but UID
// can't be overriden by the user, so it's the only field that can be
// used for internal differentiation of the users.
// UID is not exported in the metrics or traces.
// UID uniquely identifies a service instance. It is not exported
// in the metrics or traces, but it is used to compose the InstanceID
UID UID

Name string
Expand All @@ -71,7 +69,6 @@ type ID struct {
// again with Kubernetes metadata).
Namespace string
SDKLanguage InstrumentableType
Instance string

Metadata map[attr.Name]string

Expand Down
35 changes: 7 additions & 28 deletions pkg/internal/traces/read_decorator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ type InstanceIDConfig struct {
// value. Beyla will anyway attach the process ID to the given hostname for composing
// the instance ID.
OverrideHostname string `yaml:"override_hostname" env:"BEYLA_HOSTNAME"`
// OverrideInstanceID can be optionally set to avoid Beyla composing the instance ID
// and use this value. If you are managing multiple processes from a single Beyla instance,
// all the processes will have the same Instance ID.
OverrideInstanceID string `yaml:"override_instance_id" env:"BEYLA_INSTANCE_ID"`

// Undocumented properties aimed at fine-grained tuning

Expand All @@ -55,7 +51,7 @@ type ReadDecorator struct {
type decorator func(spans []request.Span)

func ReadFromChannel(ctx context.Context, r *ReadDecorator) pipe.StartFunc[[]request.Span] {
decorate := getDecorator(&r.InstanceID)
decorate := hostNamePIDDecorator(&r.InstanceID)
return func(out chan<- []request.Span) {
cancelChan := ctx.Done()
for {
Expand All @@ -76,30 +72,14 @@ func ReadFromChannel(ctx context.Context, r *ReadDecorator) pipe.StartFunc[[]req
}
}

func getDecorator(cfg *InstanceIDConfig) decorator {
hnPidDecorator := hostNamePIDDecorator(cfg)
if cfg.OverrideInstanceID == "" {
return hnPidDecorator
}
return func(spans []request.Span) {
// first decorate normally
hnPidDecorator(spans)
// later, override instance IDs
for i := range spans {
spans[i].ServiceID.Instance = cfg.OverrideInstanceID
}
}
}

func hostNamePIDDecorator(cfg *InstanceIDConfig) decorator {
// TODO: periodically update in case the current Beyla instance is created from a VM snapshot running as a different hostname
resolver := hostname.CreateResolver(cfg.OverrideHostname, "", cfg.HostnameDNSResolution)
fullHostName, _, err := resolver.Query()
log := rlog().With("function", "instance_ID_hostNamePIDDecorator")
if err != nil {
log.Warn("can't read hostname. Leaving empty. Consider overriding"+
" the BEYLA_HOSTNAME or BEYLA_INSTANCE_ID properties",
"error", err)
" the BEYLA_HOSTNAME property", "error", err)
} else {
log.Info("using hostname", "hostname", fullHostName)
}
Expand All @@ -109,17 +89,16 @@ func hostNamePIDDecorator(cfg *InstanceIDConfig) decorator {
if cfg.InternalIDCacheLen != 0 {
cacheLen = cfg.InternalIDCacheLen
}
idsCache, _ := lru.New[uint32, string](cacheLen)
uidsCache, _ := lru.New[uint32, svc.UID](cacheLen)

return func(spans []request.Span) {
for i := range spans {
instanceID, ok := idsCache.Get(spans[i].Pid.HostPID)
uid, ok := uidsCache.Get(spans[i].Pid.HostPID)
if !ok {
instanceID = fullHostName + "-" + strconv.Itoa(int(spans[i].Pid.HostPID))
idsCache.Add(spans[i].Pid.HostPID, instanceID)
uid = svc.UID(fullHostName + "-" + strconv.Itoa(int(spans[i].Pid.HostPID)))
uidsCache.Add(spans[i].Pid.HostPID, uid)
}
spans[i].ServiceID.Instance = instanceID
spans[i].ServiceID.UID = svc.UID(instanceID)
spans[i].ServiceID.UID = uid
spans[i].ServiceID.HostName = fullHostName
}
}
Expand Down
16 changes: 2 additions & 14 deletions pkg/internal/traces/read_decorator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,35 +27,23 @@ func TestReadDecorator(t *testing.T) {
type testCase struct {
desc string
cfg ReadDecorator
expectedID string
expectedUID svc.UID
expectedHN string
}
for _, tc := range []testCase{{
desc: "dns",
cfg: ReadDecorator{InstanceID: InstanceIDConfig{HostnameDNSResolution: true}},
expectedID: dnsHostname + "-1234",
expectedUID: svc.UID(dnsHostname + "-1234"),
expectedHN: dnsHostname,
}, {
desc: "no-dns",
expectedID: localHostname + "-1234",
expectedUID: svc.UID(localHostname + "-1234"),
expectedHN: localHostname,
}, {
desc: "override hostname",
cfg: ReadDecorator{InstanceID: InstanceIDConfig{OverrideHostname: "foooo"}},
expectedID: "foooo-1234",
expectedUID: "foooo-1234",
expectedHN: "foooo",
}, {
desc: "override HN",
cfg: ReadDecorator{InstanceID: InstanceIDConfig{OverrideInstanceID: "instanceee"}},
expectedID: "instanceee",
// even if we override instance ID, the UID should be set to a really unique value
// (same as the automatic instanceID value)
expectedUID: svc.UID(localHostname + "-1234"),
expectedHN: localHostname,
}} {
t.Run(tc.desc, func(t *testing.T) {
cfg := tc.cfg
Expand All @@ -72,9 +60,9 @@ func TestReadDecorator(t *testing.T) {
}
outSpans := testutil.ReadChannel(t, decoratedOutput, testTimeout)
assert.Equal(t, []request.Span{
{ServiceID: svc.ID{Instance: tc.expectedID, UID: tc.expectedUID, HostName: tc.expectedHN},
{ServiceID: svc.ID{UID: tc.expectedUID, HostName: tc.expectedHN},
Path: "/foo", Pid: request.PidInfo{HostPID: 1234}},
{ServiceID: svc.ID{Instance: tc.expectedID, UID: tc.expectedUID, HostName: tc.expectedHN},
{ServiceID: svc.ID{UID: tc.expectedUID, HostName: tc.expectedHN},
Path: "/bar", Pid: request.PidInfo{HostPID: 1234}},
}, outSpans)
})
Expand Down
4 changes: 4 additions & 0 deletions pkg/transform/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ func (md *metadataDecorator) appendMetadata(span *request.Span, info *kube.PodIn
if span.ServiceID.Namespace == "" {
span.ServiceID.Namespace = info.Namespace
}
// overriding the UID here will avoid reusing the OTEL resource reporter
// if the application/process was discovered and reported information
// before the kubernetes metadata was available
// (related issue: /~https://github.com/grafana/beyla/issues/1124)
span.ServiceID.UID = svc.UID(info.UID)

// if, in the future, other pipeline steps modify the service metadata, we should
Expand Down

0 comments on commit ecc9dc1

Please sign in to comment.