Skip to content

[exporter/datadog] Expose source provider#37668

Merged
mx-psi merged 36 commits into
open-telemetry:mainfrom
mackjmr:mackjmr/expose-sourceprovider
Feb 11, 2025
Merged

[exporter/datadog] Expose source provider#37668
mx-psi merged 36 commits into
open-telemetry:mainfrom
mackjmr:mackjmr/expose-sourceprovider

Conversation

@mackjmr

@mackjmr mackjmr commented Feb 4, 2025

Copy link
Copy Markdown
Member

Description

The goal of this PR is to expose the GetSourceProvider method from hostmetadata, which is currently under an internal folder in datadogexporter.

In order to do so, we create new package github.com/open-telemetry/opentelemetry-collector-contrib/pkg/datadog/hostmetadata which calls on internal hostmetadata's GetSourceProvider.

We also create new module github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog, where we store the internal hostmetadata, clientutil and scrub packages. We move hostmetadata there as it needs to be imported in both datadogexporter, and pkg/datadog/hostmetadata, and we move clientutil and scrub there as they are imported by hostmetadata.

Link to tracking issue

Fixes

Testing

Documentation

Comment thread connector/datadogconnector/go.mod Outdated
@songy23

songy23 commented Feb 4, 2025

Copy link
Copy Markdown
Member

This warrants an API changelog too

@mackjmr

mackjmr commented Feb 5, 2025

Copy link
Copy Markdown
Member Author

CI failures will be solved once #37499 is merged

@mackjmr

mackjmr commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

We already have pkg/datadog, why create internal/datadog rather than reusing pkg/datadog?

@songy23 If we only use pkg/datadog, then we need to expose 10-15 funcs/types from clientutils / scrub / hostmetadata so they can be imported in datadogexporter. Creating internal/datadog allows us to keep them fully internal, and be able to import them in both pkg/datadog/hostmetadata and datadogexporter. It reduces surface API + need to create "exposed" packages for both clientutils / scrub.

@songy23

songy23 commented Feb 10, 2025

Copy link
Copy Markdown
Member

need to run make generate and fix lint

Comment thread .chloggen/mackjmr_expose-sourceprovider.yaml Outdated
mackjmr and others added 3 commits February 10, 2025 16:51

@songy23 songy23 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe you need to add metadata.yaml to internal/datadog, like this one https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/datadog/metadata.yaml
then run make generate again

@mackjmr

mackjmr commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

Thanks @songy23, added metadata.yaml. I think this is all thats needed, because there is no changes after make generate

@songy23

songy23 commented Feb 10, 2025

Copy link
Copy Markdown
Member

verifyAllModulesInSet failed: Module github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog (defined in /home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/datadog/go.mod) is not listed in any module set.

@songy23

songy23 commented Feb 10, 2025

Copy link
Copy Markdown
Member

You're still missing something (not sure what exactly is missing). make generate should have added internal/datadog to https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/.github/CODEOWNERS but currently it is not present

@mackjmr

mackjmr commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

Yeah, noticed that. Will try running make generate agaiin

@mackjmr

mackjmr commented Feb 10, 2025

Copy link
Copy Markdown
Member Author

@songy23 make generate didn't add it, so i added manually

Comment thread pkg/datadog/hostmetadata/hostmetadata.go
Comment thread .chloggen/mackjmr_expose-sourceprovider.yaml Outdated
@mx-psi mx-psi merged commit f985803 into open-telemetry:main Feb 11, 2025
@github-actions github-actions Bot added this to the next release milestone Feb 11, 2025
khushijain21 pushed a commit to khushijain21/opentelemetry-collector-contrib that referenced this pull request Feb 14, 2025
#### Description

The goal of this PR is to expose the `GetSourceProvider` method from
hostmetadata, which is currently under an internal folder in
datadogexporter.

In order to do so, we create new package
`github.com/open-telemetry/opentelemetry-collector-contrib/pkg/datadog/hostmetadata`
which calls on internal hostmetadata's `GetSourceProvider`.

We also create new module
`github.com/open-telemetry/opentelemetry-collector-contrib/internal/datadog`,
where we store the internal `hostmetadata`, `clientutil` and `scrub`
packages. We move `hostmetadata` there as it needs to be imported in
both datadogexporter, and `pkg/datadog/hostmetadata`, and we move
`clientutil` and `scrub` there as they are imported by `hostmetadata`.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes

<!--Describe what testing was performed and which tests were added.-->
#### Testing

<!--Describe the documentation added.-->
#### Documentation

<!--Please delete paragraphs that you did not use before submitting.-->

---------

Co-authored-by: Yang Song <songy23@users.noreply.github.com>
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.

5 participants