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

support jaeger.tags when calling 'GetSamplingStrategy', e.g. return R… #1687

Closed
wants to merge 1 commit into from

Conversation

limfriend
Copy link

…ATE_LIMITING when jaeger.tags = {region,CA}

Signed-off-by: osfriend [email protected]

Which problem is this PR solving?

SamplingStrategy support for agent level tag
e.g. return RATE_LIMITING when jaeger.tags = {"region","CA"}

Short description of the changes

transparent metadata which converted from 'jaeger.tags' to collector service

…ATE_LIMITING when jaeger.tags = {region,CA}

Signed-off-by: osfriend <[email protected]>
@limfriend
Copy link
Author

@pavolloffay @jpkrohling could you help to review this PR, I have not idea about CI build failure.

@yurishkuro
Copy link
Member

Could you please first open a ticket and describe the business use case that you're trying to solve?

@limfriend
Copy link
Author

@yurishkuro add issue for this PR, #1692

@yurishkuro
Copy link
Member

Thanks. So it looks like this PR only passes the agent tags as metadata to the collector, it does not actually implement being able to vary sampling strategies in response to those tags.

@limfriend
Copy link
Author

limfriend commented Jul 26, 2019

your are right, It is easy to write code for passing tags to collector . but It is complex to get sampling strategies for composing serviceName and agentTag.

  • Maybe some agent tags is global for sampling strategies. e.g samplingRate=1 when t 'environment' tag= test.
  • maybe some serviceName is ignore agent tags for sampling strategies. e.g ‘order’ service is always samplingRate=1, No matter what the 'environment' tag is test or product.

I have no clean idea about how to config the priority for serviceName and agentTag. Could you please give me some help?

@yurishkuro
Copy link
Member

There is a related proposal here #365 (comment), which will require representing sampling rules with more granularity than just service-operation as today. That approach is easily generalizable to include agent tags.

@limfriend
Copy link
Author

yes, it is a good generalizable proposal. When is it probably done? could you please merge the agent code first, then I extended agentTag Sampling base this sampling rules.

@yurishkuro
Copy link
Member

I don't know if @lookfwd made any progress on it. We internally have similar requirements and will likely work on that functionality in September. But we haven't finalized the backend design yet.

I don't want to merge this PR until we have a design for the backend, because it's not guaranteed that we will want to keep the data path in this form. We can keep it open for now.

@jpkrohling
Copy link
Contributor

What's the status of this PR?

@jpkrohling jpkrohling closed this Oct 27, 2020
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.

4 participants