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

Allow specification of excluded endpoints for tracing+metrics #953

Closed
wants to merge 1 commit into from

Conversation

hughsimpson
Copy link
Contributor

Being able to exclude specific endpoints from tracing+metrics tracking can reduce noise (e.g. I would like to be able to exclude health & metrics endpoints in my apps). Might be desirable to permit distinguishing 'exclude from metrics' from 'exclude from tracing' but for now I've assumed that they would be similar concerns, and aggregated them into a single 'excludes' list

@hughsimpson
Copy link
Contributor Author

hughsimpson commented Mar 7, 2021

Failing tests appear to be entirely unrelated. See other pr for some fixes

(#952)

@hughsimpson
Copy link
Contributor Author

On reflection, splitting the two is probably better

@SimunKaracic
Copy link
Contributor

This looks interesting, but it's touching some core stuff, so we'll need to take a closer look at it.
In the meantime, can you add one test that verifies this solution?

@hughsimpson
Copy link
Contributor Author

Sure. This one was a little tentative anyway, will update pr tomorrow

@pnerg
Copy link
Contributor

pnerg commented Mar 9, 2021

I achieved the same/similar behaviour by overriding the sampling decision.
This way I could get rid of useless incoming requests (e.g. health) as well as outgoing requests such as lookups in Consul.

Custom sampler:
In my case I wrap the RandomSampler by proxying it with a filter.
If we pass my filter then we use the RandomSampler. As it was how I wanted it to work.

/**
  * Custom decision point if the span for a particular URI/path is to be sampled
  */
class PathFilterSampler extends Sampler with PathFilter {
  
  // uses the random sampler in case we pass the URI filtering
  // configured using the same means as if the sampler would be created by Kamon itself
  // https://github.com/kamon-io/Kamon/blob/master/kamon-core/src/main/scala/kamon/trace/Tracer.scala#L363
  private val sampler = RandomSampler(Kamon.config().getDouble("kamon.trace.random-sampler.probability"))

  override def decide(operation: Sampler.Operation): Trace.SamplingDecision = {
    if(accept(operation.operationName())) sampler.decide(operation) else Trace.SamplingDecision.DoNotSample
  }
}

Path filter utility

object PathFilter {
  /** 
    * Singleton of the filter to be used
    */
  private val filter:Option[Filter] = Try(Kamon.filter("kamon.filters.path-filter")).toOption
}

trait PathFilter {
  import PathFilter._

  /**
    * Filter the provided path/URI.
    * If no filter is configured the response defaults to ''true''
    */
  def accept(path:String):Boolean = filter.map(_.accept(path)).getOrElse(true)
}

Custom config for the filter:

kamon.filters {
  "path-filter" {
    includes = ["**"]
    #exclude calls to Consul and all non-functional paths from the span measurements and metric generation
    excludes = ["regex:.*/v1/.*", "regex:.*/app-details", "regex:.*/health", "regex:.*/health/.*-status"]
  }
}

Override the sampler:

kamon.trace.sampler = "foo.bar.kamon.PathFilterSampler"

@hughsimpson
Copy link
Contributor Author

Yeah that looks like a nicer solution, will give it a try and probably close this pr

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.

3 participants