Add support for all rule conditions to be applied per-span

[sozuuuuu]

I am currently creating a sampling rule to use refinery in production. I created the following rule to exclude health checks against my_server.

[ds]

	Sampler = "RulesBasedSampler"

	[[ds.rule]]
		name = "drop healtchecks"
		drop = true
		[[ds.rule.condition]]
			field = "http.target"
			operator = "="
			value = "/health-check"

However, unintentionally all the traces like below were dropped.

The reason is that the rule traverses all spans and applies the rule if it matches the given condition. In this case, my_server uses the health check endpoint of my_server2.

I have added a condition to the rule to fix this.

[ds]

	Sampler = "RulesBasedSampler"

	[[ds.rule]]
		name = "drop healtchecks"
		drop = true
		[[ds.rule.condition]]
			field = "trace.parent_id"
			operator = "not-exists"
		[[ds.rule.condition]]
			field = "http.target"
			operator = "="
			value = "/health-check"

However, due to the logic I mentioned earlier, such a trace would be dropped.

Is there a way to set up the filter that "the filter is applied to the traces that contain at least one span that matches the n conditions" rather than "given n conditions, if the trace contains n spans that match the condition, apply the filter". If not, I would appreciate if you support this.

[MikeGoldsmith]

Hi @sozuuuuu

Thanks for creating the issue. I'm not sure how we'd work around this, I'll check with some other folks who are better versed in setting up more complicated rule sets and report back.

[MikeGoldsmith]

Hi @sozuuuuu - I'm sorry for the delay in getting back to you, we've been really busy the past few weeks.

Have you found a workaround for the issue?

[sozuuuuu]

I have not found a workaround. Perhaps if I were to write such a sampling rule, I would create my own Sampler class.

This logic is useful when auto instrumentation creates traces with unintended root spans.

Trace1 Unintended:

[span0|root] db.system=redis, name=GET

Trace2:

[span0|root] name=rack
[span1] db.system=redis, name=GET

If I write such a rule, even Trace2 will be dropped.
Because this has 2 rules. The rule1 matches Trace2.root and the rule2 matches Trace2.span1. So the number of matches count is 2.

[ds]

	Sampler = "RulesBasedSampler"

	[[ds.rule]]
		name = "drop unintended redis calls"
		drop = true
		[[ds.rule.condition]]
			field = "trace.parent_id"
			operator = "not-exists"
		[[ds.rule.condition]]
			field = "db.system"
			operator = "="
			value = "redis"

[MikeGoldsmith]

Hey, sorry again for delay in getting back to you.

Refinery intentionally always applies all conditions, and looks for a match in any of the trace's spans -- this is by design.

An option to work around this might be to include a known field in the traces to filter on, or maybe if you have a discrete number of controllers (eg UserController). This way you could disqualify /health-checks when they either contain a field you known to have set in traces you want to capture, or includes a span from a known controller.

Do you think that might work for you?

[isnotajoke]

FWIW, this is also something we'd like to be able to do. For context, we have a number of microservices logging to a single dataset in Honeycomb, and we use the rules-based sampler to sample these services. These services sometimes interact with each other: we often see traces that start at service A and then make calls to services B, C, D, etc.

These services can see dramatically different load levels between them, and we'd like to be able to sample per-service – it would be hard to accurately and predictably account for differing request volume with a single across the board sampling setting.

What we'd like to be able to do is something like:

• For traces originating at service X, sample at 10
• For traces originating at service Y, sample at 100
• ...

The issue noted above makes this difficult – it's not easy to confidently identify "Traces originating at service X" or "Traces originating at service Y". Our first attempt was something like:

[[dataset.rule]]
  name = "service X traces"
  SampleRate = 100
  [[dataset.rule.condition]]
  field = "meta.span_type"
  operator = "="
  value = "root"
  [[dataset.rule.condition]]
  field = "service_name"
  operator = "="
  value = "X"

If this rule were only satisfied if both conditions were satisfied by a single span, we'd be able to do what we want. As written, the rules-based sampler effectively makes the first condition (matching a root span) a no-op (some span within any trace should be the root span, barring errors), leaving only the second condition. By itself, service_name = X is too broad – it will match traces originating at service X, but also traces originating elsewhere that happen to call service X at some point.

If we were able to add an attribute to a rule telling refinery that it needs to be evaluated per-span (and not per-trace), I think that would help us do what we want. For example:

[[dataset.rule]]
  name = "service X traces"
  SampleRate = 100
  MatchSpan = true
  [[dataset.rule.condition]]
  field = "meta.span_type"
  operator = "="
  value = "root"
  [[dataset.rule.condition]]
  field = "service_name"
  operator = "="
  value = "X"

MatchSpan = true could tell refinery that each of the conditions needs to match the same span in a trace for the rule to be matched. If that setting is false (or omitted), we get the current behavior.

I hacked together a quick proof of concept of how this could look in code. Not sure if this is something you're interested in. It does add a fair amount of complexity to the rule sampler, so I could see why it wouldn't be desirable. I'd be happy to polish up that MVP a bit and open a PR if it is something you'd consider incorporating in refinery, though.

[MikeGoldsmith]

Hey - thanks for the write up and proof of concept @isnotajoke.

I like the general idea of introducing a rule property to require all conditions match a single span instead of any span within the trace. We would be interested in exploring this further if you're willing to create a PR and we'll help where we can.

[isnotajoke]

Thanks @MikeGoldsmith ! I'll polish that up and submit it as a PR when it's ready.

[MikeGoldsmith]

Fixed by #440. A new release will be processed soon.