Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Reduce memory allocations in HttpExporter #21

Merged
merged 1 commit into from
Feb 3, 2023
Merged

Reduce memory allocations in HttpExporter #21

merged 1 commit into from
Feb 3, 2023

Conversation

cd21h
Copy link
Contributor

@cd21h cd21h commented Jan 11, 2023

Use ArrayPool instead of new buffer allocation when sending logs to OpenTelemetry collector.

@nblumhardt
Copy link
Member

Looks great to me, but I'm not familiar with CodedOutputStream, which the original code uses but the replacement doesn't. Is this intended, or an oversight? Thanks!

@cd21h
Copy link
Contributor Author

cd21h commented Jan 20, 2023

This was done on purpose since CodedOutputStream does not support Span

@nblumhardt
Copy link
Member

Thanks for the follow-up! Was the encoding performed by CodedOutputStream unnecessary in this case?

@loomis-relativity
Copy link
Contributor

The encoding doesn't do anything in this case. PR LGTM. Merging...

@loomis loomis merged commit 7ad91b6 into serilog:dev Feb 3, 2023
@loomis
Copy link
Contributor

loomis commented Feb 3, 2023

Thanks for the contribution.

@loomis loomis mentioned this pull request Feb 9, 2023
Merged
@nblumhardt nblumhardt mentioned this pull request May 18, 2023
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cd21h @nblumhardt @loomis-relativity @loomis