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

Added parameterizedSampling.pdf write-up for Discussion on Parameterized or targeted Sampling and Upstream Filtering #224

Closed
wants to merge 2 commits into from

Conversation

falenn
Copy link

@falenn falenn commented Aug 20, 2019

All -

I'd like to float concepts for parameterized sampling and have included a write-up to that effect. thanks in advance for your grace and please let me know if in any way I can help. The spec is a PDF document, parameterizedSampling.pdf that I included at ~/opentelemetry-specification/work_in_progress/specification/trace/parameterizedSampling.pdf

@yurishkuro
Copy link
Member

yurishkuro commented Aug 20, 2019

@falenn could you publish it in some other (resizable) format, like a Word or Google doc? Even on the iPad the font is like 1mm tall.

@bogdandrutu
Copy link
Member

I cannot see the file.

@songy23
Copy link
Member

songy23 commented Aug 20, 2019

Please sign the CLA.

File can be viewed at https://github.com/open-telemetry/opentelemetry-specification/blob/c9d9fec22c37e91eca54d15241aa9b35115c85a6/work_in_progress/specification/trace/parameterizedSampling.pdf. However as @yurishkuro suggests consider putting the content in a md file similar to other specs. Also note that work_in_progress directory will be removed in #225.

@falenn
Copy link
Author

falenn commented Aug 20, 2019

Thank you fro the feedback. I will reformat. Where would you recommend I place this write-up? I can do it as markdown or Google doc.

@yurishkuro
Copy link
Member

@falenn as the first quick step, I would suggest a Google doc, to kick off discussion. At 14 pages I am really interested to read it (we're working on ad-hoc sampling at Uber right now as well). I don't care at this point if it becomes markdown, since you have tables & charts, probably not worth it initially.

@falenn
Copy link
Author

falenn commented Aug 21, 2019

Thank you for your patience! I've now reformatted the doc and put it in Google Docs https://docs.google.com/document/d/1QohEjBkOafo6GdlQHhWK6_CU0UpIZ9NqKePErsAOfHE/edit?usp=sharing

@bputt
Copy link

bputt commented Aug 27, 2019

@yurishkuro @bogdandrutu @songy23 have you had a chance to read to google doc yet? If so, what are your thoughts?

@tedsuo
Copy link
Contributor

tedsuo commented Oct 4, 2019

Hi @falenn, sorry for the slow reply, just getting to reading this.

In general, I am excited about rules-based approaches like this. I see this as something beyond just sampling - an observability control plane, similar in architecture to the kinds of network switching control planes that you see out there (eg OpenFlow).

However, I feel this is a bit beyond what we are trying to ship in the initial version of OpenTelemetry. I would suggest returning with this proposal once we have a more stable version of the API, and have shipped a basic tracer. In fact, because of the layered nature of OpenTelemetry, it should be possible to implement a spiked SDK of this as a way of working out the details, while reusing the API and ecosystem.

At that point, an RFC could be made to add something like this. Possibly by making the SDK sufficiently modular to add something like this an optional plugin.

Does that feel like a reasonable approach to you?

@falenn
Copy link
Author

falenn commented Oct 4, 2019

@tedsuo, thanks for the comment, no worries! I agree with you - tunable sampling (targeted and filtered sampling) increase the fidelity of the control of sampling and therefore are extensions to the fundamental intent of distributed tracing. In an implementation I built on top of OpenCensus, I had need of extension-points prior to making the sampling decision. Particularly in the case of filtering, baggage is desirable for passing not just vendoring info, but the application namespace (in our case, the collector is multi-tenant). An RFC could address these issues as you suggest. Thanks!

@falenn
Copy link
Author

falenn commented Oct 4, 2019

@tedsuo...and yes, this does elude to an observability control plane as you suggest. I'll look into OpenFlow.

@yurishkuro
Copy link
Member

@falenn fyi we had a similar rules-based proposal for sampling in Jaeger (jaegertracing/jaeger#365 (comment)), and actually recently implemented some foundational changes in the Node.js client to support that model (e.g. https://github.com/jaegertracing/jaeger-client-node/blob/master/src/samplers/experimental/priority_sampler.js).

@SergeyKanzhelev
Copy link
Member

Closing this PR based on conversation. Let's find an alternative venue for this discussion, excited to learn more!

TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this pull request Oct 15, 2020
* Don't call nullptr->f().
* Avoid signed overflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants