Skip to content

Conversation

@martinkuba
Copy link
Contributor

Which problem is this PR solving?

As discussed in open-telemetry/opentelemetry-specification#3869, the decision has been made in the Specification SIG meeting on 3/2/24 to rename EventEmitter to EventLogger in the API.

Technically this is a breaking change. But the API is in very early experimental stage without a default SDK implementation.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@martinkuba martinkuba requested a review from a team March 21, 2024 17:05
@codecov
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Merging #4568 (31d6c75) into main (97af8e6) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4568   +/-   ##
=======================================
  Coverage   92.84%   92.84%           
=======================================
  Files         328      328           
  Lines        9494     9494           
  Branches     2040     2040           
=======================================
  Hits         8815     8815           
  Misses        679      679           
Files Coverage Δ
...imental/packages/api-events/src/NoopEventLogger.ts 100.00% <100.00%> (ø)
...packages/api-events/src/NoopEventLoggerProvider.ts 100.00% <100.00%> (ø)
experimental/packages/api-events/src/api/events.ts 100.00% <100.00%> (ø)
...l/packages/api-events/src/internal/global-utils.ts 100.00% <ø> (ø)

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

@martinkuba renaming looks good, but looks like there's still a lot of places where the term event emitter is used in the context of the api-events package.

@pichlermarc
Copy link
Member

(re-running failed test, seems to be unrelated)

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 🙂

@pichlermarc pichlermarc merged commit f6a075b into open-telemetry:main Mar 25, 2024
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
…4568)

* renamed EventEmitter to EventLogger

* updated changelog

* renamed remaining references to emitter

* lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants