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.

Sign up for GitHub

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

Jump to bottom

[exporter/datadog] Add configurable reporter_period for host metadata #36451

Merged
merged 9 commits into from
Dec 3, 2024
Merged

[exporter/datadog] Add configurable reporter_period for host metadata #36451

merged 9 commits into from
Dec 3, 2024

Conversation

NassimBtk
Copy link
Contributor

Description

This pull request introduces a new configurable parameter reporter_period for the Datadog exporter’s host metadata configuration. This enhancement allows users to specify the frequency at which host metadata is sent to Datadog. The changes span multiple files and include updates to configuration, factory methods, and test cases.

Enhancements to Datadog exporter:

Configuration updates:

Test case updates:

  • exporter/datadogexporter/factory_test.go, exporter/datadogexporter/logs_exporter_test.go, exporter/datadogexporter/metrics_exporter_test.go, exporter/datadogexporter/traces_exporter_test.go: Updated test cases to include the new reporter_period parameter. [1] [2] [3] [4]
  • pkg/datadog/config/config_test.go: Added test cases to validate the reporter_period configuration. [1] [2]

Link to tracking issue

Fixes #36450

Testing

Documentation

@NassimBtk
[exporter/datadog] Add configurable reporter_period for host metadata
fe7cdff 
- Introduced a `reporter_period` field in `HostMetadataConfig` to allow
customization of the metadata reporting interval.
- Default value set to 30 minutes for backward compatibility.
- Updated `pkg/datadog/config/host.go` to support the new
`reporter_period` field.
- Validated `reporter_period` in `pkg/datadog/config/config.go` to
ensure positive durations.
- Passed `reporter_period` to the metadata pusher in
`exporter/datadogexporter/internal/hostmetadata/config.go`.
- Modified `exporter/datadogexporter/hostmetadata.go` and `factory.go`
to integrate `reporter_period` into the reporter logic.
- Removed hardcoded constant for metadata reporting interval in favor of
the configurable value.

Fixes open-telemetry#36450
@NassimBtk
[exporter/datadog] Add configurable reporter_period for host metadata
5588e25 
- Added a changelog entry
@NassimBtk NassimBtk requested review from mx-psi, songy23 and a team as code owners November 19, 2024 17:20
@jade-guiton-dd jade-guiton-dd mentioned this pull request Nov 25, 2024
Closed
jade-guiton-dd
jade-guiton-dd suggested changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@jade-guiton-dd jade-guiton-dd left a comment

Choose a reason for hiding this comment

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

Everything looks good to me, just one small change. I'll probably take care of following up with #36522 once this is merged.

pkg/datadog/config/config.go Outdated
Comment on lines 136 to 137
if c.HostMetadata.ReporterPeriod <= 0 {
return errors.New("reporter_period must be a positive duration")
Copy link
Contributor

Choose a reason for hiding this comment

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

We would like to prevent users from accidentally spamming the backend by setting the reporter period too low. The Agent currently has a minimum of 5 minutes, so unless you believe there to be strong use cases for even lower values, I think we should do the same here.

Suggested change
if c.HostMetadata.ReporterPeriod <= 0 {
return errors.New("reporter_period must be a positive duration")
if c.HostMetadata.ReporterPeriod < 5 * time.Minute {
return errors.New("reporter_period must be 5 minutes or higher")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes sense, we can go with minimum 5 minutes. It's already very good like this. Thank you!

jade-guiton-dd
jade-guiton-dd approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@jade-guiton-dd jade-guiton-dd left a comment

Choose a reason for hiding this comment

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

LGTM!

@mx-psi mx-psi merged commit 5002476 into open-telemetry:main Dec 3, 2024
158 checks passed
@github-actions github-actions bot added this to the next release milestone Dec 3, 2024
shivanthzen pushed a commit to shivanthzen/opentelemetry-collector-contrib that referenced this pull request Dec 5, 2024
@NassimBtk @shivanthzen
[exporter/datadog] Add configurable reporter_period for host metadata (…
a2b357e 
…open-telemetry#36451)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This pull request introduces a new configurable parameter
`reporter_period` for the Datadog exporter’s host metadata
configuration. This enhancement allows users to specify the frequency at
which host metadata is sent to Datadog. The changes span multiple files
and include updates to configuration, factory methods, and test cases.

Enhancements to Datadog exporter:

*
[`.chloggen/add-configurable-reporter-period-for-host-metadata.yaml`](diffhunk://#diff-e3e3c57c49d2a921abc8470bccafabf4d632b0dc142eb3e95b3b6fac57107a11R1-R27):
Added a changelog entry for the new `reporter_period` parameter.
*
[`exporter/datadogexporter/examples/collector.yaml`](diffhunk://#diff-766be9823b8ec8b8de17e2c6c785d9f93e13b5a7e84ff441040077ab5e726a79R452-R456):
Added documentation for the new `reporter_period` parameter in the
example configuration file.
*
[`exporter/datadogexporter/factory.go`](diffhunk://#diff-c9e9f39ffda08a2af74c0d54d37f21e14cdb95c4136cdbe238de6be2bfe31389L127-R125):
Updated the factory to use the `reporter_period` from the configuration
instead of a hardcoded value.

Configuration updates:

*
[`exporter/datadogexporter/internal/hostmetadata/config.go`](diffhunk://#diff-0c9f0862d61390add366aca3e30834a1e01f16a3865ef9aca61831cad3459f53R31-R32):
Added `ReporterPeriod` to the `PusherConfig` struct.
*
[`pkg/datadog/config/config.go`](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139):
Added validation for the `reporter_period` to ensure it is a positive
duration and set a default value.
[[1]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139)
[[2]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R347)

Test case updates:

* `exporter/datadogexporter/factory_test.go`,
`exporter/datadogexporter/logs_exporter_test.go`,
`exporter/datadogexporter/metrics_exporter_test.go`,
`exporter/datadogexporter/traces_exporter_test.go`: Updated test cases
to include the new `reporter_period` parameter.
[[1]](diffhunk://#diff-c50b6d1f06d62a58b6422f3218526437c16554faa2b7fc8eee1d32c021f00ca4R311)
[[2]](diffhunk://#diff-94f5b0119d06dbcfee20cd9b99a104ef208e027c166c463321670a5197e83714R233-R235)
[[3]](diffhunk://#diff-005adb1774d2a402bdd7adf3b6aa150117bf6b3c0bb634bf2c68c6b33f0ce1b5L61-R63)
[[4]](diffhunk://#diff-c19daad040b7310787c0012e5c660dc18a07ba16036d0eaa31a0e18c551d0ef3R148-R150)
*
[`pkg/datadog/config/config_test.go`](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105):
Added test cases to validate the `reporter_period` configuration.
[[1]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105)
[[2]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R186-R197)
<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36450

<!--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.-->
ZenoCC-Peng pushed a commit to ZenoCC-Peng/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2024
@NassimBtk @ZenoCC-Peng
[exporter/datadog] Add configurable reporter_period for host metadata (…
c0016ac 
…open-telemetry#36451)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This pull request introduces a new configurable parameter
`reporter_period` for the Datadog exporter’s host metadata
configuration. This enhancement allows users to specify the frequency at
which host metadata is sent to Datadog. The changes span multiple files
and include updates to configuration, factory methods, and test cases.

Enhancements to Datadog exporter:

*
[`.chloggen/add-configurable-reporter-period-for-host-metadata.yaml`](diffhunk://#diff-e3e3c57c49d2a921abc8470bccafabf4d632b0dc142eb3e95b3b6fac57107a11R1-R27):
Added a changelog entry for the new `reporter_period` parameter.
*
[`exporter/datadogexporter/examples/collector.yaml`](diffhunk://#diff-766be9823b8ec8b8de17e2c6c785d9f93e13b5a7e84ff441040077ab5e726a79R452-R456):
Added documentation for the new `reporter_period` parameter in the
example configuration file.
*
[`exporter/datadogexporter/factory.go`](diffhunk://#diff-c9e9f39ffda08a2af74c0d54d37f21e14cdb95c4136cdbe238de6be2bfe31389L127-R125):
Updated the factory to use the `reporter_period` from the configuration
instead of a hardcoded value.

Configuration updates:

*
[`exporter/datadogexporter/internal/hostmetadata/config.go`](diffhunk://#diff-0c9f0862d61390add366aca3e30834a1e01f16a3865ef9aca61831cad3459f53R31-R32):
Added `ReporterPeriod` to the `PusherConfig` struct.
*
[`pkg/datadog/config/config.go`](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139):
Added validation for the `reporter_period` to ensure it is a positive
duration and set a default value.
[[1]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139)
[[2]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R347)

Test case updates:

* `exporter/datadogexporter/factory_test.go`,
`exporter/datadogexporter/logs_exporter_test.go`,
`exporter/datadogexporter/metrics_exporter_test.go`,
`exporter/datadogexporter/traces_exporter_test.go`: Updated test cases
to include the new `reporter_period` parameter.
[[1]](diffhunk://#diff-c50b6d1f06d62a58b6422f3218526437c16554faa2b7fc8eee1d32c021f00ca4R311)
[[2]](diffhunk://#diff-94f5b0119d06dbcfee20cd9b99a104ef208e027c166c463321670a5197e83714R233-R235)
[[3]](diffhunk://#diff-005adb1774d2a402bdd7adf3b6aa150117bf6b3c0bb634bf2c68c6b33f0ce1b5L61-R63)
[[4]](diffhunk://#diff-c19daad040b7310787c0012e5c660dc18a07ba16036d0eaa31a0e18c551d0ef3R148-R150)
*
[`pkg/datadog/config/config_test.go`](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105):
Added test cases to validate the `reporter_period` configuration.
[[1]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105)
[[2]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R186-R197)
<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36450

<!--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.-->
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this pull request Dec 17, 2024
@NassimBtk @sbylica-splunk
[exporter/datadog] Add configurable reporter_period for host metadata (…
101ec6e 
…open-telemetry#36451)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This pull request introduces a new configurable parameter
`reporter_period` for the Datadog exporter’s host metadata
configuration. This enhancement allows users to specify the frequency at
which host metadata is sent to Datadog. The changes span multiple files
and include updates to configuration, factory methods, and test cases.

Enhancements to Datadog exporter:

*
[`.chloggen/add-configurable-reporter-period-for-host-metadata.yaml`](diffhunk://#diff-e3e3c57c49d2a921abc8470bccafabf4d632b0dc142eb3e95b3b6fac57107a11R1-R27):
Added a changelog entry for the new `reporter_period` parameter.
*
[`exporter/datadogexporter/examples/collector.yaml`](diffhunk://#diff-766be9823b8ec8b8de17e2c6c785d9f93e13b5a7e84ff441040077ab5e726a79R452-R456):
Added documentation for the new `reporter_period` parameter in the
example configuration file.
*
[`exporter/datadogexporter/factory.go`](diffhunk://#diff-c9e9f39ffda08a2af74c0d54d37f21e14cdb95c4136cdbe238de6be2bfe31389L127-R125):
Updated the factory to use the `reporter_period` from the configuration
instead of a hardcoded value.

Configuration updates:

*
[`exporter/datadogexporter/internal/hostmetadata/config.go`](diffhunk://#diff-0c9f0862d61390add366aca3e30834a1e01f16a3865ef9aca61831cad3459f53R31-R32):
Added `ReporterPeriod` to the `PusherConfig` struct.
*
[`pkg/datadog/config/config.go`](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139):
Added validation for the `reporter_period` to ensure it is a positive
duration and set a default value.
[[1]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R136-R139)
[[2]](diffhunk://#diff-747858d4c8fea28a92227df09f17ebee01df63dad04812ac5d30a392c9970b73R347)

Test case updates:

* `exporter/datadogexporter/factory_test.go`,
`exporter/datadogexporter/logs_exporter_test.go`,
`exporter/datadogexporter/metrics_exporter_test.go`,
`exporter/datadogexporter/traces_exporter_test.go`: Updated test cases
to include the new `reporter_period` parameter.
[[1]](diffhunk://#diff-c50b6d1f06d62a58b6422f3218526437c16554faa2b7fc8eee1d32c021f00ca4R311)
[[2]](diffhunk://#diff-94f5b0119d06dbcfee20cd9b99a104ef208e027c166c463321670a5197e83714R233-R235)
[[3]](diffhunk://#diff-005adb1774d2a402bdd7adf3b6aa150117bf6b3c0bb634bf2c68c6b33f0ce1b5L61-R63)
[[4]](diffhunk://#diff-c19daad040b7310787c0012e5c660dc18a07ba16036d0eaa31a0e18c551d0ef3R148-R150)
*
[`pkg/datadog/config/config_test.go`](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105):
Added test cases to validate the `reporter_period` configuration.
[[1]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R97-R105)
[[2]](diffhunk://#diff-d3a3c19e78bbac07597c6210b4d051f4678df2752e06c82d0f040c52207c5970R186-R197)
<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes open-telemetry#36450

<!--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.-->
@NassimBtk NassimBtk mentioned this pull request Dec 17, 2024
Closed
6 tasks
@NassimBtk NassimBtk deleted the feature/issue-36450-datadogexporter-add-configurable-reporter-period-for-host-metadata branch December 23, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jade-guiton-dd jade-guiton-dd jade-guiton-dd approved these changes

@mx-psi mx-psi mx-psi approved these changes

@ankitpatel96 ankitpatel96 Awaiting requested review from ankitpatel96

@dineshg13 dineshg13 Awaiting requested review from dineshg13

@liustanley liustanley Awaiting requested review from liustanley

@mackjmr mackjmr Awaiting requested review from mackjmr

@songy23 songy23 Awaiting requested review from songy23 songy23 is a code owner

Assignees

@djaglowski djaglowski

Labels
exporter/datadog Datadog components pkg/datadog
Projects
None yet
Milestone
v0.115.0
Development

Successfully merging this pull request may close these issues.

[exporter/datadog] Add Configurable Reporter Period for Host Metadata
5 participants
@NassimBtk @mx-psi @songy23 @jade-guiton-dd @djaglowski