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

[Feature]: SimpleSpanProcessor to be consistent with SimpleLogRecordProcessor #1409

Closed
cijothomas opened this issue Nov 28, 2023 · 4 comments · Fixed by #1612
Closed

[Feature]: SimpleSpanProcessor to be consistent with SimpleLogRecordProcessor #1409

cijothomas opened this issue Nov 28, 2023 · 4 comments · Fixed by #1612
Labels
enhancement New feature or request triage:todo Needs to be traiged.

Comments

@cijothomas
Copy link
Member

Related Problems?

No response

Describe the solution you'd like:

#1308 modified SimpleLogRecordProcessor to simply take a Mutex protected call to Exporter.
Proposing to make the similar change to SimpleSpanProcessor, effectively reversing #502.

We'll get to eliminate the crossbeam-channel by doing this, and also ensures there is more consistency between signals.

Considered Alternatives

No response

Additional Context

No response

@cijothomas cijothomas added enhancement New feature or request triage:todo Needs to be traiged. labels Nov 28, 2023
@jtescher
Copy link
Member

This may simplify things, but we should check the overall throughput to make sure it's still acceptable given our performance focus (e.g. if an exporter has an average latency of ~1 second)

@cijothomas
Copy link
Member Author

I don't think the perf would be good here (and will directly be affected by how slow the exporter is!). For performance scenarios - there is BatchProcessors to the rescue, so my main point was - we should not worry about perf for SimpleProcessor, which should only be used for testing/learning purposes!

@cijothomas
Copy link
Member Author

Any further comments/objections to do this?

@lalitb
Copy link
Member

lalitb commented Feb 22, 2024

One of the concerns I believe was users needing performance in hot-path without depending on the external runtime. This should be resolved with #1506.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage:todo Needs to be traiged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants