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/logging] Remove logging exporter #11037

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

TylerHelmuth
Copy link
Member

@TylerHelmuth TylerHelmuth commented Sep 3, 2024

Description

It is September, which means we get to remove this deprecated component.

Link to tracking issue

Related to #7769

Copy link

codecov bot commented Sep 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.44%. Comparing base (9f57a55) to head (692bcfc).
Report is 13 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #11037   +/-   ##
=======================================
  Coverage   91.44%   91.44%           
=======================================
  Files         424      422    -2     
  Lines       20270    20202   -68     
=======================================
- Hits        18535    18473   -62     
+ Misses       1359     1354    -5     
+ Partials      376      375    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mx-psi
mx-psi previously requested changes Sep 4, 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.

There are a bunch of references on examples in language library repositories still that mention the logging exporter. This search has a lot of false positives, but it's a good start I think: https://github.com/search?q=org%3Aopen-telemetry+%22logging%3A%22+-repo%3Aopen-telemetry%2Fopentelemetry-collector+language%3AYAML+OR+language%3AMarkdown&type=code&p=1

We also have ~18% of survey respondents from the Collector survey raw data using the logging exporter so I think we need to do this very carefully.

At a minimum I feel like we need a migration page, and a very clear error message that tells you to use the debug exporter and links you to the migration page. Additionally, it would be great to have something like #7631 before we do this (not sure if it should be a hard requirement tho).

I am blocking this until we at least have no references to the logging exporter within examples in the OpenTelemetry org

codeboten pushed a commit that referenced this pull request Sep 4, 2024
#### Description

Add warning to logging exporter on startup to remind users to switch to
the debug exporter.

#### Link to tracking issue
Related to
#11037

#### Testing

![image](https://github.com/user-attachments/assets/7d9cd908-6fdf-45c4-82f0-56cce35dbcee)
codeboten added a commit to codeboten/opentelemetry-java-examples that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-go-contrib that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037
codeboten added a commit to codeboten/opentelemetry-java-contrib that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-php that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-android that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-cpp-contrib that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
codeboten added a commit to codeboten/opentelemetry-erlang-contrib that referenced this pull request Sep 20, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
@TylerHelmuth TylerHelmuth requested a review from a team as a code owner September 24, 2024 15:11
@TylerHelmuth TylerHelmuth force-pushed the remove-logging-exporter branch 2 times, most recently from c541232 to eb4045a Compare September 24, 2024 15:15
@TylerHelmuth
Copy link
Member Author

@mx-psi it has been another 4 weeks with an even louder warning and @codeboten has updated a majority of the loggingexporter references in language repos. I feel we should move forward with this removal.

dmathieu pushed a commit to open-telemetry/opentelemetry-go-contrib that referenced this pull request Sep 25, 2024
This exporter has been replaced by the debug exporter and will be
removed soon. Related to
open-telemetry/opentelemetry-collector#11037
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.

I've opened PRs to most of the remaining references to the logging exporter found in the query that were not in archived repositories.

I don't know that it's worth waiting for all of them to be merged before moving this forward.

@mx-psi mx-psi dismissed their stale review October 1, 2024 10:00

Language repos have been updated

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.

I don't think it should block this PR (we can do this in a separate one) but I feel like we should give a clearer error message when trying to use the logging exporter for a while.

For example, we can do the following: if a configuration file references a logging exporter and the logging exporter is not available, but the debug exporter is, then add a hint to the error message suggesting the usage of the debug exporter.

Based on the survey usage data, I expect this change to be quite disruptive for users and for us to get quite a few Slack messages and Github issues about this, so I would like to help users as much as possible

@TylerHelmuth
Copy link
Member Author

@mx-psi added an additional warning:

receivers:
  otlp:
    protocols:
      grpc:
exporters:
  logging:
    level: debug
service:
  pipelines:
    traces:
      receivers:
        - otlp
      exporters:
        - logging
❯ ./bin/otelcorecol_darwin_arm64 --config ./bin/config.yaml
Error: failed to get config: cannot unmarshal the configuration: decoding failed due to the following error(s):

error decoding 'exporters': the logging exporter has been deprecated, use the debug exporter instead
2024/10/01 08:55:31 collector server run finished with error: failed to get config: cannot unmarshal the configuration: decoding failed due to the following error(s):

error decoding 'exporters': the logging exporter has been deprecated, use the debug exporter instead

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.

IMO this is ready to merge once we remove the exporter from the opentelemetry-collector-releases manifests

@TylerHelmuth
Copy link
Member Author

brettmc pushed a commit to brettmc/opentelemetry-php that referenced this pull request Oct 2, 2024
This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>
bobstrecansky pushed a commit to open-telemetry/opentelemetry-php that referenced this pull request Oct 2, 2024
* 8.4: fixing some implicit nullable params

* fix withspan handler nullable + example (#1377)

- class is nullable for pre hooks
- add phpt tests for withspan and its interaction with auto root span

* fix: update references to logging exporter (#1383)

This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>

* fix: Update collector tag in example to latest (#1384)

Follow-up from #1383. Debug exporter isn't available until v0.86.0, so this new configuration throws an error when trying to run the collector. `Error: cannot unmarshal the configuration: unknown exporters type "debug" for "debug"` Instead of setting to v0.86.0, I noticed the other docker files omitted the tag (so latest is pulled). I just updated this to match.

* Remove `MeterInterface::isEnabled()` and fix meter config re-enabling (#1387)

* Update SPI dependency to v1. (#1388)

* Fix README badges. (#1389)

* add phpDocumentor instructions to Makefile (#1385)

* add phpDocumentor instructions to Makefile

Signed-off-by: svrnm <[email protected]>

* update DEVELOPMENT.md

Signed-off-by: svrnm <[email protected]>

---------

Signed-off-by: svrnm <[email protected]>

* Update README.md (#1344)

* Fix `IncompatibleReturnValueException` in `MessageFactoryTest` (#1392)

PSR7 `RequestInterface::getUri()` must return `UriInterface`, not `string`; explicit typehint was added in `2.0`.

* more implicit nulls

---------

Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: svrnm <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
Co-authored-by: Jamie Danielson <[email protected]>
Co-authored-by: Tobias Bachert <[email protected]>
Co-authored-by: Chris Lightfoot-Wild <[email protected]>
Co-authored-by: Severin Neumann <[email protected]>
Co-authored-by: Oleg <[email protected]>
otel-php-bot pushed a commit to opentelemetry-php/config-sdk that referenced this pull request Oct 2, 2024
* 8.4: fixing some implicit nullable params

* fix withspan handler nullable + example (#1377)

- class is nullable for pre hooks
- add phpt tests for withspan and its interaction with auto root span

* fix: update references to logging exporter (#1383)

This exporter has been replaced by the debug exporter and will be removed soon. Related to open-telemetry/opentelemetry-collector#11037

Signed-off-by: Alex Boten <[email protected]>

* fix: Update collector tag in example to latest (#1384)

Follow-up from #1383. Debug exporter isn't available until v0.86.0, so this new configuration throws an error when trying to run the collector. `Error: cannot unmarshal the configuration: unknown exporters type "debug" for "debug"` Instead of setting to v0.86.0, I noticed the other docker files omitted the tag (so latest is pulled). I just updated this to match.

* Remove `MeterInterface::isEnabled()` and fix meter config re-enabling (#1387)

* Update SPI dependency to v1. (#1388)

* Fix README badges. (#1389)

* add phpDocumentor instructions to Makefile (#1385)

* add phpDocumentor instructions to Makefile

Signed-off-by: svrnm <[email protected]>

* update DEVELOPMENT.md

Signed-off-by: svrnm <[email protected]>

---------

Signed-off-by: svrnm <[email protected]>

* Update README.md (#1344)

* Fix `IncompatibleReturnValueException` in `MessageFactoryTest` (#1392)

PSR7 `RequestInterface::getUri()` must return `UriInterface`, not `string`; explicit typehint was added in `2.0`.

* more implicit nulls

---------

Signed-off-by: Alex Boten <[email protected]>
Signed-off-by: svrnm <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
Co-authored-by: Jamie Danielson <[email protected]>
Co-authored-by: Tobias Bachert <[email protected]>
Co-authored-by: Chris Lightfoot-Wild <[email protected]>
Co-authored-by: Severin Neumann <[email protected]>
Co-authored-by: Oleg <[email protected]>
@mx-psi mx-psi added the release:blocker The issue must be resolved before cutting the next release label Oct 3, 2024
@mx-psi
Copy link
Member

mx-psi commented Oct 3, 2024

I pinned issue #11337 which we can use as the central place for any people that need help with the migration. We can keep it pinned and open for a couple weeks and close it after a while.

I think this has enough approvals to merge so I am merging, thanks everyone for the hard work, let's hope this goes smoothly!

@mx-psi mx-psi merged commit f9d44de into open-telemetry:main Oct 3, 2024
66 checks passed
@github-actions github-actions bot added this to the next release milestone Oct 3, 2024
jackgopack4 pushed a commit to jackgopack4/opentelemetry-collector that referenced this pull request Oct 8, 2024
#### Description

It is September, which means we get to remove this deprecated component.

- The helm chart was updated to exclude the logging exporter already:
https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-collector/UPGRADING.md#0840-to-0850.
- The Operator references have been updated:
open-telemetry/opentelemetry-operator#3259.
- Opentelemetry.io site final references to update:
open-telemetry/opentelemetry.io#5143

<!-- Issue number if applicable -->
#### Link to tracking issue
Related to
open-telemetry#7769
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:blocker The issue must be resolved before cutting the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants