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

[exporter/datadog] Upgrade exporter.datadogexporter.UseLogsAgentExporter feature flag to beta #34420

Merged
merged 6 commits into from
Aug 14, 2024

Conversation

liustanley
Copy link
Contributor

@liustanley liustanley commented Aug 6, 2024

Description:

Use Datadog Agent logs pipeline by default for exporting logs to Datadog. Upgrades exporter.datadogexporter.UseLogsAgentExporter feature flag to beta.

Note: if users have logs::dump_payloads enabled in their config after upgrading, an error will be raised because this config option cannot be used with the logs agent exporter:

{setting: "logs::dump_payloads", valid: !isLogsAgentExporterEnabled()},

Link to tracking Issue:
Fixes #34366 and introduces general performance improvements.

Testing:

Documentation:

@liustanley liustanley requested a review from a team August 6, 2024 15:21
@github-actions github-actions bot added the exporter/datadog Datadog components label Aug 6, 2024
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

One nit, LGTM otherwise

exporter/datadogexporter/factory_test.go Show resolved Hide resolved
@mx-psi
Copy link
Member

mx-psi commented Aug 7, 2024

Note: if users have logs::dump_payloads enabled in their config after upgrading, an error will be raised because this config option cannot be used with the logs agent exporter:

This is a breaking change and we should list it in the changelog.

Do we intend to provide this functionality in the future? Or is this just not going to be supported?

@liustanley
Copy link
Contributor Author

Note: if users have logs::dump_payloads enabled in their config after upgrading, an error will be raised because this config option cannot be used with the logs agent exporter:

This is a breaking change and we should list it in the changelog.

Do we intend to provide this functionality in the future? Or is this just not going to be supported?

I don't think there's a corresponding feature/config option in the logs agent, so we were going to leave it unless someone wants to request it as a feature. I believe logs::dump_payloads is only used in debugging.

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@songy23 songy23 added the on hold This is blocked by another PR/issue label Aug 7, 2024
@songy23
Copy link
Member

songy23 commented Aug 7, 2024

On hold until 0.108 release as discussed offline

mx-psi pushed a commit that referenced this pull request Aug 8, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Deprecates `logs::dump_payloads` because this config option is invalid
with the Datadog Agent logs pipeline which will be enabled by default in
the following release. This PR will be included in v0.107.0 and
#34420
will be included in v0.108.0.

**Link to tracking Issue:** <Issue number if applicable>

**Testing:** <Describe what testing was performed and which tests were
added.>
The log appears as follows:
```
2024-08-07T14:49:10.514-0400    warn    datadogexporter@v0.106.1/config.go:462  logs::dump_payloads is deprecated and will raise an error if set when the Datadog Agent logs pipeline is enabled by default in collector version v0.108.0    {"kind": "exporter", "data_type": "logs", "name": "datadog"}
```

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Yang Song <songy23@users.noreply.github.com>
mx-psi
mx-psi previously requested changes Aug 8, 2024
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Blocking merge until v0.108.0

@mx-psi mx-psi dismissed their stale review August 14, 2024 12:14

v0.107.0 has been released

@mx-psi
Copy link
Member

mx-psi commented Aug 14, 2024

@liustanley can you fix the merge conflict?

@songy23 songy23 removed the on hold This is blocked by another PR/issue label Aug 14, 2024
@mx-psi mx-psi merged commit ebaf5cc into open-telemetry:main Aug 14, 2024
154 checks passed
@mx-psi mx-psi deleted the stanley.liu/logs-agent-beta branch August 14, 2024 14:49
@github-actions github-actions bot added this to the next release milestone Aug 14, 2024
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024
)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Deprecates `logs::dump_payloads` because this config option is invalid
with the Datadog Agent logs pipeline which will be enabled by default in
the following release. This PR will be included in v0.107.0 and
open-telemetry#34420
will be included in v0.108.0.

**Link to tracking Issue:** <Issue number if applicable>

**Testing:** <Describe what testing was performed and which tests were
added.>
The log appears as follows:
```
2024-08-07T14:49:10.514-0400    warn    datadogexporter@v0.106.1/config.go:462  logs::dump_payloads is deprecated and will raise an error if set when the Datadog Agent logs pipeline is enabled by default in collector version v0.108.0    {"kind": "exporter", "data_type": "logs", "name": "datadog"}
```

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Yang Song <songy23@users.noreply.github.com>
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024
…rter` feature flag to beta (open-telemetry#34420)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Use Datadog Agent logs pipeline by default for exporting logs to
Datadog. Upgrades `exporter.datadogexporter.UseLogsAgentExporter`
feature flag to beta.

Note: if users have `logs::dump_payloads` enabled in their config after
upgrading, an error will be raised because this config option cannot be
used with the logs agent exporter:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/f1aadc6c9c5d117eb61898cd2f1969a279a7f80e/exporter/datadogexporter/config.go#L658

**Link to tracking Issue:** <Issue number if applicable>
Fixes
open-telemetry#34366
and introduces general performance improvements.

**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/datadog Datadog components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[exporter/datadog] incorrect service.name transformation
4 participants