Fix: Remove hardcoded sampling-strategies.json dependency in Jaeger in-one binary

[yunmaoQu]

Which problem is this PR solving?

• fix [bug]: v2 all-in-one binary cannot find sampling strategies file #6338

Description of the changes

• Removed the hardcoded path in the default configuration (all-in-one.yaml).
• Updated the code to log a warning if the file is missing and proceed with default behavior.
• Deleted the unused sampling-strategies.json file.

How was this change tested?

• passed

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: npm run lint and npm run test

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.05%. Comparing base (e98f7f5) to head (29c1ab5).
Report is 57 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (e98f7f5) and HEAD (29c1ab5). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (e98f7f5)	HEAD (29c1ab5)
unittests	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #6356       +/-   ##
===========================================
- Coverage   96.07%   49.05%   -47.03%     
===========================================
  Files         359      182      -177     
  Lines       20504    11205     -9299     
===========================================
- Hits        19700     5497    -14203     
- Misses        613     5262     +4649     
- Partials      191      446      +255     

Flag	Coverage Δ
badger_v1	8.84% <ø> (ø)
badger_v2	1.60% <ø> (ø)
cassandra-4.x-v1-manual	14.74% <ø> (ø)
cassandra-4.x-v2-auto	1.55% <ø> (ø)
cassandra-4.x-v2-manual	1.55% <ø> (ø)
cassandra-5.x-v1-manual	14.74% <ø> (ø)
cassandra-5.x-v2-auto	1.55% <ø> (ø)
cassandra-5.x-v2-manual	1.55% <ø> (ø)
elasticsearch-6.x-v1	18.36% <ø> (ø)
elasticsearch-7.x-v1	18.45% <ø> (+<0.01%)	⬆️
elasticsearch-8.x-v1	18.60% <ø> (ø)
elasticsearch-8.x-v2	1.60% <ø> (ø)
grpc_v1	10.34% <ø> (+<0.01%)	⬆️
grpc_v2	7.81% <ø> (ø)
kafka-v1	9.17% <ø> (ø)
kafka-v2	1.60% <ø> (ø)
memory_v2	1.60% <ø> (ø)
opensearch-1.x-v1	18.49% <ø> (-0.01%)	⬇️
opensearch-2.x-v1	18.50% <ø> (ø)
opensearch-2.x-v2	1.60% <ø> (+<0.01%)	⬆️
tailsampling-processor	0.45% <ø> (ø)
unittests	?

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]

this logic does not belong in main

[yurishkuro]

We still want to use a file based strategy by default. A user running all-in-one should be able to override the path via --set flag.

[avinpy-255]

Hi @yurishkuro
I tried running the Jaeger binary locally and read the error message (same as in the GitHub comments). I think the issue is that it cannot find the sampling-strategies.json file. I believe defining the file path for the sampling strategies in the config file might resolve the issue, but I'm not sure.

I also checked the config files under cmd/jaeger/internal/config.yaml and found this line, which is commented out:

remote_sampling:      
# You can either use file or adaptive sampling strategy in remote_sampling      
# file:      
#   path: ./cmd/jaeger/sampling-strategies.json   

I'm wondering why this is commented out. Is this the file where I should make changes?

[yurishkuro]

It's not commented out

jaeger/cmd/jaeger/internal/all-in-one.yaml

Lines 30 to 34 in 4ecb086

	remote_sampling:
	# We can either use file or adaptive sampling strategy in remote_sampling
	file:
	path: ./cmd/jaeger/sampling-strategies.json
	# adaptive:

But it is incorrect to leave it like this because it assumes some random location of the config. My proposal would be to keep the file: strategy selected, but comment out the file name. This will fail the validatin so we'd need to fix that first, then in the actual strategy implementation it should allow empty file name and return a default strategy (probabilistic with p=1.0), because that would be equivalent to our existing config but without the bogus service names.

[yurishkuro]

Closing this in favor of #6431