[jaeger-v2] Add StartTimeMin and StartTimeMax params to consumeTraces function.

[james-ryans]

Which problem is this PR solving?

• Fixes [jaeger-v2] add Badger storage backend integration test #5281 issue for not being able to consume traces

Description of the changes

• Added StartTimeMin and StartTimeMax query params with 1h lookback to consumeTraces function.
• Added logging to the data receivers with the OTelCol codes below as the references.
  • https://github.com/open-telemetry/opentelemetry-collector/blob/main/otelcol/unmarshaler.go#L45
  • https://github.com/open-telemetry/opentelemetry-collector/blob/main/service/telemetry/telemetry.go#L118

How was this change tested?

• Tested manually.

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.09%. Comparing base (c56e724) to head (755dc7e).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5288      +/-   ##
==========================================
+ Coverage   95.08%   95.09%   +0.01%     
==========================================
  Files         340      340              
  Lines       16610    16626      +16     
==========================================
+ Hits        15793    15811      +18     
+ Misses        629      627       -2     
  Partials      188      188              

Flag	Coverage Δ
badger	13.26% <ø> (ø)
cassandra-3.x	26.44% <ø> (ø)
cassandra-4.x	26.44% <ø> (ø)
elasticsearch-5.x	21.70% <ø> (ø)
elasticsearch-6.x	21.68% <ø> (-0.02%)	⬇️
elasticsearch-7.x	21.78% <ø> (ø)
elasticsearch-8.x	21.85% <ø> (-0.02%)	⬇️
grpc	11.00% <100.00%> (+0.08%)	⬆️
kafka	14.73% <ø> (ø)
opensearch-1.x	21.77% <ø> (ø)
opensearch-2.x	21.78% <ø> (+0.01%)	⬆️
unittests	92.28% <30.76%> (-0.03%)	⬇️

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]

Did you also mean to add any additional logging in the datareceiver? Or does it do the logging already and you just weren't giving it a good logger?

[james-ryans]

Did you also mean to add any additional logging in the datareceiver? Or does it do the logging already and you just weren't giving it a good logger?

Yes, it already has the logging there. This is where the logs have captured the error from Badger's integration test.

jaeger/cmd/jaeger/internal/integration/receivers/storagereceiver/receiver.go

Lines 74 to 78 in d887b74

	for _, svc := range services {
	if err := r.consumeTraces(ctx, svc); err != nil {
	r.settings.Logger.Error("Failed to consume traces from consumer", zap.Error(err))
	}
	}

[yurishkuro]

Any reason why can't pass tel directly?

[james-ryans]

Because tel (or telemetry.Telemetry) and component.TelemetrySettings is two different component.
telemetry.Telemetry initialize the zap.Logger from the config.
component.TelemetrySettings is the struct that contains zap.Logger that is used by components in OTEL to report telemetry matters.

This means that the extension and receiver in the data receiver are using component.TelemetrySettings to write logs. Based on your suggestion, I thought it is better if we pass the initialized struct and use it directly in the data receiver.

[yurishkuro]

Got it. Looking through OTEL code the situation seems to be quite messy, I don't know if they have a ticket to clean it up. E.g.

• why are Telemetry and TelemetrySettings two different structs? They seem to server the same purpose (the latter being more comprehensive)
• TelemetrySettings comment also mentions yet another similar struct servicetelemetry.TelemetrySettings

[yurishkuro]

Yes, it already has the logging there. This is where the logs have captured the error from Badger's integration test.

I was rather thinking of INFO logs as the data receiver polls the storage - it's a test scenario, we don't have to be stingy about logs. E.g. something like

• querying for traces, params: ...
• retrieved N spans
• sleeping till the next retry