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.

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

Recommeded resource attributes #3797

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .chloggen/recommedded-resource-attributes.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: auto-instrumentation

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Update recommended resource attributes to match the [semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more info about the exact changes that need to be done for the operator.

Also, provide some kind of recommendation regarding upgrading.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the subtext has all changes - is that sufficient?

The subtext should also cover what you need to consider for upgrading.


# One or more tracking issues related to the change
issues: [3797]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
This change updates the recommended resource attributes to match the [semantic conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md).
The following attributes have been updated:
- `service.version` now uses the docker image digest in addition to the tag
- the well-known label `app.kubernetes.io/part-of` has been removed
- the well-known label `app.kubernetes.io/instance` has been added (translates to `service.name`)

7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,9 @@ EOF

## Configure resource attributes

The OpenTelemetry Operator can automatically set resource attributes as defined in the
[OpenTelemetry Semantic Conventions](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md).

### Configure resource attributes with annotations

This example shows a pod configuration with OpenTelemetry annotations using the `resource.opentelemetry.io/` prefix.
Expand All @@ -752,12 +755,12 @@ spec:

### Configure resource attributes with labels

You can also use common labels to set resource attributes.
You can also use common labels to set resource attributes (first entry wins).

The following labels are supported:
- `app.kubernetes.io/instance` becomes `service.name`
- `app.kubernetes.io/name` becomes `service.name`
- `app.kubernetes.io/version` becomes `service.version`
- `app.kubernetes.io/part-of` becomes `service.namespace`

```yaml
apiVersion: v1
Expand Down
3 changes: 2 additions & 1 deletion apis/v1alpha1/instrumentation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,10 @@ type Sampler struct {
// Defaults defines default values for the instrumentation.
type Defaults struct {
// UseLabelsForResourceAttributes defines whether to use common labels for resource attributes:
// Note: first entry wins:
// - `app.kubernetes.io/instance` becomes `service.name`
// - `app.kubernetes.io/name` becomes `service.name`
// - `app.kubernetes.io/version` becomes `service.version`
// - `app.kubernetes.io/part-of` becomes `service.namespace`
UseLabelsForResourceAttributes bool `json:"useLabelsForResourceAttributes,omitempty"`
}

Expand Down
5 changes: 3 additions & 2 deletions docs/api/instrumentations.md
Original file line number Diff line number Diff line change
Expand Up @@ -1382,9 +1382,10 @@ Defaults defines default values for the instrumentation.
<td>boolean</td>
<td>
UseLabelsForResourceAttributes defines whether to use common labels for resource attributes:
Note: first entry wins:
- `app.kubernetes.io/instance` becomes `service.name`
- `app.kubernetes.io/name` becomes `service.name`
- `app.kubernetes.io/version` becomes `service.version`
- `app.kubernetes.io/part-of` becomes `service.namespace`<br/>
- `app.kubernetes.io/version` becomes `service.version`<br/>
</td>
<td>false</td>
</tr></tbody>
Expand Down
12 changes: 8 additions & 4 deletions pkg/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ const (
AnnotationDefaultAutoInstrumentationApacheHttpd = InstrumentationPrefix + "default-auto-instrumentation-apache-httpd-image"
AnnotationDefaultAutoInstrumentationNginx = InstrumentationPrefix + "default-auto-instrumentation-nginx-image"

LabelAppName = "app.kubernetes.io/name"
LabelAppVersion = "app.kubernetes.io/version"
LabelAppPartOf = "app.kubernetes.io/part-of"

LabelTargetAllocator = "opentelemetry.io/target-allocator"
ResourceAttributeAnnotationPrefix = "resource.opentelemetry.io/"

Expand All @@ -51,3 +47,11 @@ const (
TACollectorTLSKeyFileName = "tls.key"
TACollectorTLSCertFileName = "tls.crt"
)

var (
LabelAppName = []string{
"app.kubernetes.io/instance",
"app.kubernetes.io/name",
}
LabelAppVersion = []string{"app.kubernetes.io/version"}
)
57 changes: 47 additions & 10 deletions pkg/instrumentation/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ package instrumentation

import (
"context"
"errors"
"fmt"
"sort"
"strings"
"time"
"unsafe"

"github.com/distribution/reference"
"github.com/go-logr/logr"
"go.opentelemetry.io/otel/attribute"
semconv "go.opentelemetry.io/otel/semconv/v1.7.0"
Expand Down Expand Up @@ -431,13 +433,15 @@ func chooseServiceName(pod corev1.Pod, useLabelsForResourceAttributes bool, reso
// The precedence is as follows:
// 1. annotation with key resource.opentelemetry.io/<resource>.
// 2. label with key labelKey.
func chooseLabelOrAnnotation(pod corev1.Pod, useLabelsForResourceAttributes bool, resource attribute.Key, labelKey string) string {
func chooseLabelOrAnnotation(pod corev1.Pod, useLabelsForResourceAttributes bool, resource attribute.Key, labelKeys []string) string {
if v := pod.GetAnnotations()[(constants.ResourceAttributeAnnotationPrefix + string(resource))]; v != "" {
return v
}
if useLabelsForResourceAttributes {
if v := pod.GetLabels()[labelKey]; v != "" {
return v
for _, labelKey := range labelKeys {
if v := pod.GetLabels()[labelKey]; v != "" {
return v
}
}
}
return ""
Expand All @@ -452,13 +456,46 @@ func chooseServiceVersion(pod corev1.Pod, useLabelsForResourceAttributes bool, i
if v != "" {
return v
}
parts := strings.Split(pod.Spec.Containers[index].Image, ":")
tag := parts[len(parts)-1]
//guard statement to handle case where image name has a port number
if strings.Contains(tag, "/") {
var err error
v, err = parseServiceVersionFromImage(pod.Spec.Containers[index].Image)
if err != nil {
return ""
}
return tag
return v
}

var cannotRetrieveImage = errors.New("cannot retrieve image name")

// parseServiceVersionFromImage parses the service version for differently-formatted image names
// according to https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/k8s-attributes.md#how-serviceversion-should-be-calculated
func parseServiceVersionFromImage(image string) (string, error) {
ref, err := reference.Parse(image)
if err != nil {
return "", err
}

namedRef, ok := ref.(reference.Named)
if !ok {
return "", cannotRetrieveImage
}
var tag, digest string
if taggedRef, ok := namedRef.(reference.Tagged); ok {
tag = taggedRef.Tag()
}
if digestedRef, ok := namedRef.(reference.Digested); ok {
digest = digestedRef.Digest().String()
}
if digest != "" {
if tag != "" {
return fmt.Sprintf("%s@%s", tag, digest), nil
}
return digest, nil
}
if tag != "" {
return tag, nil
}

return "", cannotRetrieveImage
}

// chooseServiceInstanceId returns the service.instance.id to be used in the instrumentation.
Expand All @@ -472,7 +509,7 @@ func createServiceInstanceId(pod corev1.Pod, namespaceName, podName, containerNa
// which violates the uniqueness requirement of service instance id -
// see https://opentelemetry.io/docs/specs/semconv/resource/#service-experimental.
// We still allow the user to set the service instance id via annotation, because this is explicitly set by the user.
serviceInstanceId := chooseLabelOrAnnotation(pod, false, semconv.ServiceInstanceIDKey, "")
serviceInstanceId := chooseLabelOrAnnotation(pod, false, semconv.ServiceInstanceIDKey, nil)
if serviceInstanceId != "" {
return serviceInstanceId
}
Expand Down Expand Up @@ -539,7 +576,7 @@ func (i *sdkInjector) createResourceMap(ctx context.Context, otelinst v1alpha1.I
}
}
}
partOf := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceNamespaceKey, constants.LabelAppPartOf)
partOf := chooseLabelOrAnnotation(pod, useLabelsForResourceAttributes, semconv.ServiceNamespaceKey, nil)
if partOf != "" && !existingRes[string(semconv.ServiceNamespaceKey)] {
res[string(semconv.ServiceNamespaceKey)] = partOf
}
Expand Down
Loading
Loading