[sampling] Fix merging of per-operation strategies into service strategies without them

[kuujis]

Which problem is this PR solving?

• Part 1 of [[Bug]: Operation based sampling strategies are not being returned for ratelimiting service strategies #5270]

Description of the changes

• See issue for description of the bug and of the two changed behaviors
  • Default probabilistic operation level strategies will now be returned if service strategy is of ratelimiting type
  • If the service itself has probabilistic strategy, its sampling rate takes priority for operation-level definition

How was this change tested?

• Via updated unit test
• By building local all-in-one executable and using it as a otel instrumentation backend for simple java service https://github.com/kuujis/opentelemetry-java-5504 and using the below sampling_strategies.json from JaegerRemoteSampler inconsistent Implementation open-telemetry/opentelemetry-java#5504

{
  "service_strategies": [
    {
      "service": "foobar",
      "type": "ratelimiting",
      "param": 2
    }
  ],
  "default_strategy": {
    "type": "probabilistic",
    "param": 0.2,
    "operation_strategies": [
      {
        "operation": "metrics",
        "type": "probabilistic",
        "param": 0.0
      }
    ]
  }
}

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: yarn lint and yarn test

[yurishkuro]

Sorry, it's been quite a while since this code was written, and I am not able to follow either your change or your unit test changes. I would recommend unit tests to be structured as follows:

1. define config file
2. execute request to retrieve sampling strategy for a service
3. compare it with a pre-constructed response that is easy to read

Steps 1 and 3 are best described via JSON docs under /fixtures, or embedded JSON strings

[kuujis]

Sorry, it's been quite a while since this code was written, and I am not able to follow either your change or your unit test changes. I would recommend unit tests to be structured as follows:

1. define config file
2. execute request to retrieve sampling strategy for a service
3. compare it with a pre-constructed response that is easy to read

Steps 1 and 3 are best described via JSON docs under /fixtures, or embedded JSON strings

After looking over the test - extracting expected response to separate file makes things a lot more readable :)

[kuujis]

Thank you for the comments, I will address them, but I'm a bit swamped with life things, so this gets a bit of a back seat for now :(

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.22%. Comparing base (9d69adc) to head (db54fd0).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5277      +/-   ##
==========================================
+ Coverage   95.20%   95.22%   +0.02%     
==========================================
  Files         343      343              
  Lines       16777    16813      +36     
==========================================
+ Hits        15972    16010      +38     
  Misses        606      606              
+ Partials      199      197       -2     

Flag	Coverage Δ
badger	10.47% <ø> (-0.04%)	⬇️
cassandra-3.x	18.38% <ø> (-0.06%)	⬇️
cassandra-4.x	18.38% <ø> (-0.06%)	⬇️
elasticsearch-5.x	20.89% <ø> (+<0.01%)	⬆️
elasticsearch-6.x	20.85% <ø> (-0.04%)	⬇️
elasticsearch-7.x	20.94% <ø> (-0.01%)	⬇️
elasticsearch-8.x	21.18% <ø> (+0.06%)	⬆️
grpc	14.55% <ø> (-0.05%)	⬇️
kafka	10.17% <ø> (+<0.01%)	⬆️
opensearch-1.x	21.01% <ø> (+0.02%)	⬆️
opensearch-2.x	20.98% <ø> (+<0.01%)	⬆️
unittests	91.75% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yurishkuro]

Thanks!

[yurishkuro]

please revert ui submodule change, it's not related

git checkout main
git submodule update
git checkout your-branch
git commit -asm 'fix ui'

[kuujis]

please revert ui submodule change, it's not related

git checkout main
git submodule update
git checkout your-branch
git commit -asm 'fix ui'

Done, sorry, should not have happened.

[yurishkuro]

I edited description to NOT resolve the issue, to make sure we do the follow-ups.