-
Notifications
You must be signed in to change notification settings - Fork 192
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
Remove event logger #1480
base: 2.x
Are you sure you want to change the base?
Remove event logger #1480
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 2.x #1480 +/- ##
============================================
- Coverage 72.20% 72.12% -0.08%
+ Complexity 2730 2713 -17
============================================
Files 401 395 -6
Lines 8152 8091 -61
============================================
- Hits 5886 5836 -50
+ Misses 2266 2255 -11
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
src/API/Logs/NoopLogger.php
Outdated
@@ -35,4 +36,16 @@ public function isEnabled(): bool | |||
{ | |||
return false; | |||
} | |||
|
|||
public function emitEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add @codeCoverageIgnore
to this method emitEvent
src/API/Logs/LoggerInterface.php
Outdated
/** | ||
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.40.0/specification/logs/api.md#emit-an-event | ||
*/ | ||
public function emitEvent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't add this method until it is defined by the spec.
(Was removed in open-telemetry/opentelemetry-specification#4352)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. I'll remove this portion for now. Converted to a draft to wait and see what the replacement might be.
was just removed from spec, replacement is not defined yet.
Event Logger has been removed from spec, in favour of Logger.EmitEvent. We have deprecated EventLogger in
main
, this removes it from2.x
and addsEmitEvent
as its replacement.