Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Ignores some tags when you create a span with forced sampling #738

Closed
Audrius-GR opened this issue Aug 28, 2020 · 3 comments · Fixed by #739
Closed

Ignores some tags when you create a span with forced sampling #738

Audrius-GR opened this issue Aug 28, 2020 · 3 comments · Fixed by #739

Comments

@Audrius-GR
Copy link

Audrius-GR commented Aug 28, 2020

Requirement - what kind of business use case are you trying to solve?

We have an application which we generally do not want tracing enabled, but we have certain user triggered workflows
where we'd like to enable tracing for.

The way we achieve this is for user initiated actions we build a new span (hence context) with
Tags.SAMPLING_PRIORITY set to 1, together with other tags specific to the users request.

Problem - what in Jaeger blocks you from solving the requirement?

There is no specific order in which the tags get processed and added to the context/span, as it's an iteration over a map:

for (Map.Entry<String, Object> tag : tags.entrySet()) {
setTagAsObject(tag.getKey(), tag.getValue());
}

This causes a problem, because tags are only set if the context is sampled

if (context.isSampled()) {
tags.put(key, value);
}

But the decision whether it's sampled or not comes from one of the tags, which can arrive later (due to iteration over a map)
Hence all tags prior to the sampling tag are dropped on the floor and ignored.

Proposal - what do you suggest to solve the problem or improve the existing situation?

I think it should set the tags on the context unconditionally, or explicitly check for the sampling tag and do context adjustments
before setting the rest of the tags.

Sad workaround

Create a noop root span with sampling enabled, create a child span with actual user specific tags

@yurishkuro
Copy link
Member

This has been already solved in Go and Node.js Jaeger clients via delayed sampling. It can be implemented in Java as well, of someone has the time.

@AudriusButkevicius
Copy link
Contributor

Sure, yet that's more effort than just fixing this bug.

Would you guys be happy with a PR fixing this specific issue? If yes, which way you guys prefer it to be solved (always put tags in context, check sampling tag before hand)?

@yurishkuro
Copy link
Member

Are you saying that you're supplying SAMPLING_PRIORITY at span creation, but it's still ignoring other tags due to map iteration order? If so, then yes, it sounds like a bug, this specific tag should be processed first.

AudriusButkevicius added a commit to AudriusButkevicius/jaeger-client-java that referenced this issue Aug 28, 2020
AudriusButkevicius added a commit to AudriusButkevicius/jaeger-client-java that referenced this issue Aug 28, 2020
AudriusButkevicius added a commit to AudriusButkevicius/jaeger-client-java that referenced this issue Aug 28, 2020
)

Signed-off-by: Audrius Butkevicius <audrius.butkevicius@gmail.com>
AudriusButkevicius added a commit to AudriusButkevicius/jaeger-client-java that referenced this issue Aug 28, 2020
)

Signed-off-by: Audrius Butkevicius <audrius.butkevicius@gmail.com>
yurishkuro pushed a commit that referenced this issue Aug 28, 2020
Signed-off-by: Audrius Butkevicius <audrius.butkevicius@gmail.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants