Skip to content

Allow opamp extension to include resource attributes automatically #38994

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

Merged

Conversation

jaronoff97
Copy link
Contributor

Description

Introduces a new config struct element IncludeResourceAttributes which when enabled (default is false) copies the set resource attributes from the agent into the non-identifying attributes in the agent description message.

Link to tracking issue

Fixes #37487

Testing

Unit

Documentation

n/a

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe this needs a bit of coverage in the README.md file.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Changes look ok, just one question about the go mod change

@jaronoff97 jaronoff97 requested a review from codeboten March 28, 2025 14:43
@jaronoff97 jaronoff97 requested a review from TylerHelmuth April 1, 2025 08:50
@douglascamata
Copy link
Member

@jaronoff97 I am trying this PR's changes and it seems that something's wrong and set.Resource.Attributes() is empty for me.

Here's what I am trying:

  • I added include_resource_attributes: true inside opamp.agent_description
  • I set up the system and env resource detectors.
  • I added a the env var OTEL_RESOURCE_ATTRIBUTES=douglas-test-attribute=successful to my Collector's deployment.

The attribute douglas-test-attribute was not present in the agent's non-identifying attributes.

I even added a small log to check if things were okay, but I don't see it in my logs:

	resourceAttrs := make(map[string]string, set.Resource.Attributes().Len())
	set.Resource.Attributes().Range(func(k string, v pcommon.Value) bool {
		set.Logger.Info(fmt.Sprintf("Resource attribute %s: %s", k, v.Str()))
		resourceAttrs[k] = v.Str()
		return true
	})

@jaronoff97
Copy link
Contributor Author

@douglascamata interesting... i have a test for this exact case here. How did you build the opamp extension for your collector?

@douglascamata
Copy link
Member

douglascamata commented Apr 1, 2025

@jaronoff97 I cloned your branch and built the Collector image from there. Here's a snippet of the configuration of my opamp extension:

      opamp:
        agent_description:
          include_resource_attributes: true
          non_identifying_attributes:
            k8s.node.name: ${env:KUBE_NODE_NAME}
        server:
          http:
            endpoint: ${env:OPAMP_SERVER_URL}
            headers:
              Authorization: Bearer ${env:CORALOGIX_PRIVATE_KEY}
            polling_interval: 2m

I can confirm that I see my custom attribute in my metrics pipeline, where the resource detection processor is used.

@jaronoff97
Copy link
Contributor Author

oh i see why now, you're using the resource attributes processor which adds resource attributes to the telemetry collected by the collector, but not to the collector itself. You'd need to set the resource in the service telemetry like so:

    service:
      telemetry:
        resource:
          hello: world

@douglascamata
Copy link
Member

Ahhh, I see, @jaronoff97. I am personally going after enriching the OpAMP agent attributes with the same data collected by the resource detection processor. I understand now that this is unrelated. 👍

@douglascamata
Copy link
Member

Sorry for the confusion.

@jaronoff97
Copy link
Contributor Author

No problem, as far as your issue goes I believe there's a way to do what you'd like via env vars, but that's a different topic. I believe what you can for that is somethign like this but referencing an environment variable with the ${env:MY_ENV_VAR} syntax.

@douglascamata
Copy link
Member

@jaronoff97 I will need a lot more than that. I want also all those cloud metadata attributes from AWS, Azure, etc.

@jaronoff97
Copy link
Contributor Author

@douglascamata let's continue this discussion in slack, there's some features you should be able to take advantage of. (tag me in the #otel-config-file channel in the CNCF slack)

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

I don't know what's going on with the go.mod change, but if CI passes, I assume it's good. Overall looks good to me.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks @jaronoff97

@codeboten
Copy link
Contributor

@jaronoff97 please resolve the conflicts and we can get this merged 👍🏻

@jaronoff97
Copy link
Contributor Author

@codeboten should be g2g! 🙇

@jaronoff97
Copy link
Contributor Author

resolve the conflicts, should be good for a merge again! 🙇

@TylerHelmuth TylerHelmuth merged commit fa620f1 into open-telemetry:main Apr 21, 2025
172 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 21, 2025
akshays-19 pushed a commit to akshays-19/opentelemetry-collector-contrib that referenced this pull request Apr 23, 2025
…pen-telemetry#38994)

#### Description
Introduces a new config struct element `IncludeResourceAttributes` which
when enabled (default is false) copies the set resource attributes from
the agent into the non-identifying attributes in the agent description
message.


#### Link to tracking issue
Fixes open-telemetry#37487

#### Testing
Unit


#### Documentation
n/a

---------

Co-authored-by: Evan Bradley <[email protected]>
Fiery-Fenix pushed a commit to Fiery-Fenix/opentelemetry-collector-contrib that referenced this pull request Apr 24, 2025
…pen-telemetry#38994)

#### Description
Introduces a new config struct element `IncludeResourceAttributes` which
when enabled (default is false) copies the set resource attributes from
the agent into the non-identifying attributes in the agent description
message.


#### Link to tracking issue
Fixes open-telemetry#37487

#### Testing
Unit


#### Documentation
n/a

---------

Co-authored-by: Evan Bradley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add resource attributes to the OpAMP Extension's non-identifying attrs
7 participants