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

logtest: Recorder result is on index 1 is instead of 0 #5357

Closed
pellared opened this issue May 15, 2024 · 2 comments · Fixed by #5365
Closed

logtest: Recorder result is on index 1 is instead of 0 #5357

pellared opened this issue May 15, 2024 · 2 comments · Fixed by #5365
Assignees
@dmathieu
Labels
area:logs Part of OpenTelemetry logs bug Something isn't working pkg:API Related to an API package
Milestone
v1.27.0

Comments

@pellared
Copy link
Member

Investigate why result on index 1 is recorded and not 0.

Originally posted by @pellared in open-telemetry/opentelemetry-go-contrib#5539 (comment)
@pellared
Copy link
Member Author

FYI @khushijain21

@pellared pellared mentioned this issue May 15, 2024
Merged
@pellared pellared changed the title otelzap: Result on index 1 is recorded instead of 0 logtest: Recorder result is on index 1 is instead of 0 May 15, 2024
@pellared pellared transferred this issue from open-telemetry/opentelemetry-go-contrib May 15, 2024
@pellared pellared added pkg:API Related to an API package area:logs Part of OpenTelemetry logs labels May 15, 2024
@pellared
Copy link
Member Author

@dmathieu, I noticed that the Result()[0] contains empty ScopeRecords which is stored in currentScopeRecord of the "root" Recorder. Shouldn't the "root" Recorder only act as LoggerProvider and the child recorders returned by Recorder.Logger implement log.Logger?

@pellared pellared added the bug Something isn't working label May 15, 2024
@dmathieu dmathieu mentioned this issue May 16, 2024
Merged
@dmathieu dmathieu self-assigned this May 16, 2024
@MrAlias MrAlias moved this from Todo to In Progress in Go: Logs (GA) May 16, 2024
MrAlias pushed a commit that referenced this issue May 17, 2024
@dmathieu @pellared
Split log/logtest into a recorder and a logger (#5365)
ebd0ade 
The current logtest.Recorder implementation is wrong. We have a single
`Recorder`, which acts as both a `LoggerProvider`, and a `Logger`,
making it possible to emit a log entry with the root recorder, which
shouldn't be possible with the API.

This change introduces a new private struct, `logger` that acts as the
recording logger, while `Recorder` becomes only a LoggerProvider and not
a Logger anymore.

Closes #5357.

---------

Co-authored-by: Robert Pająk <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Go: Logs (GA) May 17, 2024
@MrAlias MrAlias added this to the v1.27.0 milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmathieu dmathieu

Labels
area:logs Part of OpenTelemetry logs bug Something isn't working pkg:API Related to an API package
Projects
Go: Logs (GA)
Status: Done
Milestone
v1.27.0
Development

Successfully merging a pull request may close this issue.

Split log/logtest into a recorder and a logger dmathieu/opentelemetry-go
3 participants
@dmathieu @pellared @MrAlias