[hotROD] Upgrade hotrod to support otel tracer

[afzal442]

Which problem is this PR solving?

• Part of Upgrade HotROD example to use OpenTelemetry SDK #3380
• This PR tries to instrument the hotROD app with otel downgrading opentracing. Related feat: Added OTEL TracerProvider instead of opentracing.Tracer for hotROD example app service (Redis) #4533 (comment)
• Related to Upgrade internal use of tracing to OpenTelemetry #3381

Short description of the changes

• Upgrades hotROD application to support otel tracer

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (e40a3e5) 97.05% compared to head (5dd40a9) 97.03%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4548      +/-   ##
==========================================
- Coverage   97.05%   97.03%   -0.02%     
==========================================
  Files         301      301              
  Lines       17837    17837              
==========================================
- Hits        17311    17308       -3     
- Misses        422      424       +2     
- Partials      104      105       +1     

Flag	Coverage Δ
unittests	97.03% <ø> (-0.02%)	⬇️

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

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[afzal442]

Hi @yurishkuro, while I try to update the hotROD app to support otel tracer, logs under gRPC interceptor don't appear like before. Another thing is I wondered if I should make this application otel instrumented as per previous #4533 (comment).

[yurishkuro]

logs under gRPC interceptor don't appear like before

that's a problem. Can you investigate? Logs are being written to span via OT span.Log() API. You can check what OTEL Bridge does when it implements this method - I assume it should be translating them into OTEL Span Events. If that works fine, the next step to check is what jaeger2otel transformer is doing in otel-collector-contrib, maybe it's missing the logic to take Span Events and convert them back into Jaeger's span logs.

A good way to test this is to write a unit test.

[yurishkuro]

Please link the description back to the main tracking issue.

[yurishkuro]

Q: why are we accumulating such a large PR? Can we not fix each service independently?

[afzal442]

I see, I thought it could be done all at a time. But creating separate PR for each svc would be great though.

[yurishkuro]

as long as we validate that each PR does not break the functionality (like baggage propagation).

[yurishkuro]

This looks to be in good shape, subject to some smaller comments.

I would still prefer that this was done in multiple PRs, at minimum separating client / server changes, but at this point I am ok to merge as one PR.

The last remaining task is to test & document everything, to illustrate:

• that all spans are showing up (connecting) correctly
• document what is changing, because the number of spans is going to be different than in the original HotROD due to how http instrumentation works in OTEL
• test & document the baggage scenarios that were described in the original blog post. If memory serves, at least these:
  • session IDs are logged to customer (or SQL) span when transaction are blocking
  • two sets of resource usage metrics are generated

[yurishkuro]

Suggested change

	m, _ := baggage.NewMember("customer", customer.Name)
	bag := baggage.FromContext(ctx)
	bag, _ = bag.SetMember(m)
	m, err := baggage.NewMember("customer", customer.Name)
	if err != nil {
	eta.logger.For(ctx).Error("cannot create baggage member", zap.Error(err))
	}
	bag := baggage.FromContext(ctx)
	bag, err = bag.SetMember(m)
	if err != nil {
	eta.logger.For(ctx).Error("cannot set baggage member", zap.Error(err))
	}

[yurishkuro]

this change will log the errors. We're hitting a bug in OTEL, either the same as open-telemetry/opentelemetry-go#3685 or related. Basically, NewMember requires the value to be escaped, which is an incorrect behavior. But when I tried escaping it with url.EscapeQuery, then the baggage later is not serialized / deserialized at all.

[afzal442]

Cool! ignoring err cost me here.

[afzal442]

@yurishkuro after deep investigation, it came out working with the response in the HTTP request. e.g. Loading customer {"service": "customer", "component": "mysql", "trace_id": "63718f14ff8d04607e60d95da54784ae", "span_id": "b60a6202006008bc", "customer_id": "567"} here, when I try to use any of the items included say "span_id" or customer.ID or customerID or "mysql", I am getting baggage items out of it. Make sure the service should be "customer".

[afzal442]

Most insane. when I try to add logs into it, it's still not working. INFO customer/database.go:86 Getting customer {"service": "customer", "component": "mysql", "trace_id": "49af5501b6c5c75acfb1e69d19f1e481", "span_id": "e9e53f530cb6aacc", "customer_name": "Amazing Coffee Roasters"}

[afzal442]

after making minor changes, I got sth like below

"route.calc.by.customer.sec": {"Amazing_Coffee_Roasters": 0.498, "Japanese_Desserts": 0.486},
"route.calc.by.session.sec": {"1581": 0.498, "7406": 0.486}

[yurishkuro]

Yes, that's the bug it OTEL - users are not supposed to mangle the values like this in order to put them into baggage.

[yurishkuro]

🎉 🎉 🎉