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

fix(jaeger): split package to send big package to jaeger #6497

Merged
merged 1 commit into from
Jul 6, 2022
Merged

fix(jaeger): split package to send big package to jaeger #6497

merged 1 commit into from
Jul 6, 2022

Conversation

FANNG1
Copy link

@FANNG1 FANNG1 commented Jul 6, 2022

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

when using jaeger to track databend, I always see "jaeger: thrift agent failed with message too long" and set OTEL_BSP_MAX_EXPORT_BATCH_SIZE to a larger value does't take much affect. we could use "with_auto_split_batch" to split big packages.

@vercel
Copy link

vercel bot commented Jul 6, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
databend ⬜️ Ignored (Inspect) Jul 6, 2022 at 6:35AM (UTC)

@mergify
Copy link
Contributor

mergify bot commented Jul 6, 2022

This pull request's title is not fulfill the requirements. @sandflee please update it 🙏.

Valid format:

fix(query): fix group by string bug
  ^         ^---------------------^
  |         |
  |         +-> Summary in present tense.
  |
  +-------> Type: feat, fix, refactor, ci, build, docs, website, chore

Valid types:

  • feat: this PR introduces a new feature to the codebase
  • fix: this PR patches a bug in codebase
  • refactor: this PR changes the code base without new features or bugfix
  • ci|build: this PR changes build/testing/ci steps
  • docs|website: this PR changes the documents or websites
  • chore: this PR only has small changes that no need to record

@FANNG1 FANNG1 changed the title fix(jaeger) split package to send big package to jaeger fix(jaeger): split package to send big package to jaeger Jul 6, 2022
@mergify mergify bot added the pr-bugfix this PR patches a bug in codebase label Jul 6, 2022
@FANNG1
Copy link
Author

FANNG1 commented Jul 6, 2022

When auto split is set to true, the exporter will try to split the batch into smaller ones so that there will be minimal data loss. It will impact the performance.

enable auto split will impact the performance, but sending to jaeger is not enabled by default and not impact much, I think this is acceptable.

@sundy-li sundy-li requested a review from dantengsky July 6, 2022 07:38
Copy link
Member

@PsiACE PsiACE left a comment

Choose a reason for hiding this comment

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

lgtm, but a little deeper investigation is needed

Note that if one span is too large to export, other spans within the same batch may or may not be exported. In this case, exporter will return errors as we cannot split spans.

mergify bot added a commit that referenced this pull request Jul 6, 2022
@BohuTANG BohuTANG merged commit fa14a93 into databendlabs:main Jul 6, 2022
@FANNG1
Copy link
Author

FANNG1 commented Jul 6, 2022

lgtm, but a little deeper investigation is needed

@PsiACE , u want to know why "with_auto_split_batch" worked ?

@PsiACE
Copy link
Member

PsiACE commented Jul 6, 2022

u want to know why "with_auto_split_batch" worked ?

I am more concerned about whether we have some specific use cases that will lead to strange errors.

@FANNG1
Copy link
Author

FANNG1 commented Jul 6, 2022

I start jaeger with host network, docker run -d --network host registry-internal.corp.kuaishou.com/presto/jaegertracing, and could only show very little trace event. change OTEL_BSP_MAX_EXPORT_BATCH_SIZE not helped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants