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

[extension/jaegerencodingextension] Enhance jaegerEncodingExtension #27619

Closed
wants to merge 13 commits into from
Closed

[extension/jaegerencodingextension] Enhance jaegerEncodingExtension #27619

wants to merge 13 commits into from

Conversation

dao-jun
Copy link
Member

@dao-jun dao-jun commented Oct 11, 2023 •
edited by atoulme
Loading

Description:

1. Make jaegerEncodingExtension also implements ptrace.Unmarshal interface

2. Marshal traces in []*jaegerproto.Batch level instead of in Span level

The existing exporters exports traces in *jaegerproto.Batch level, this PR will be compatible with previous implementations.

3. Introducer readonlyLengthFieldBuffer and writeOnlyLengthFieldBuffer for trace marshal/unmarshal

Testing:
Unit tests

@dao-jun dao-jun requested a review from a team October 11, 2023 10:25
atoulme
atoulme approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

LGTM - needs a changelog.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 4, 2023

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 4, 2023
@github-actions GitHub Actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 19, 2023
@atoulme atoulme reopened this Nov 19, 2023
@atoulme atoulme removed the Stale label Nov 19, 2023
@atoulme
Copy link
Contributor

atoulme commented Nov 19, 2023

Please rebase

@dao-jun
Merge branch 'main' into dev/jaeger_extension_enhance
0cf4805 
# Conflicts:
#	extension/encoding/jaegerencodingextension/extension.go
@dao-jun
update branch
2f45801
@dao-jun
Copy link
Member Author

dao-jun commented Nov 22, 2023

Please rebase

branch updated

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 7, 2023

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 7, 2023
@github-actions GitHub Actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Dec 22, 2023
@dao-jun
Copy link
Member Author

dao-jun commented Dec 22, 2023

@atoulme @MovieStoreGuy Could you please help merge this PR？

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

@atoulme atoulme atoulme approved these changes

@MovieStoreGuy MovieStoreGuy Awaiting requested review from MovieStoreGuy MovieStoreGuy is a code owner

Assignees

@atoulme atoulme

Labels
extension/encoding/jaegerencoding extension/encoding Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dao-jun @atoulme