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

chore(dataobj): add logs tracking and encoding #15757

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

rfratto
Copy link
Member

@rfratto rfratto commented Jan 14, 2025

This commit is the equivalent of #15713 for the logs section: log records are accumulated into column builders and then flushed into a section at encoding time.

Some minor API changes have been made to sections/streams to ensure consistency with sections/logs.

@rfratto rfratto requested a review from a team as a code owner January 14, 2025 16:09
This commit is the equivalent of grafana#15713 for the logs section: log
records are accumulated into column builders and then flushed into a
section at encoding time.

Some minor API changes have been made to sections/streams to ensure
consistency with sections/logs.
if err := streamsEnc.Commit(); err != nil {
return err
}
s.Reset()
Copy link
Contributor

Choose a reason for hiding this comment

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

There's no need to reset if we error? In the error case, would we just abort completely and not re-use this object?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I was originally considering intermittent errors, but you're right; that shouldn't happen from the caller's perspective. I'll have it reset always.

@rfratto rfratto merged commit 11714a7 into grafana:main Jan 15, 2025
59 checks passed
@rfratto rfratto deleted the dataobj-logs branch January 15, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants