feat: Support environment-service pair to map to traceproviders

[thugrock7]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.75%. Comparing base (1fd91b9) to head (895af69).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
+ Coverage   59.37%   59.75%   +0.37%     
==========================================
  Files          72       72              
  Lines        2895     2902       +7     
==========================================
+ Hits         1719     1734      +15     
+ Misses       1095     1088       -7     
+ Partials       81       80       -1     

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

[varkey98]

Looking through the pr, I don't think we would need to change traceable or hypertrace goagent APIs. Can't we just manage this from TPA itself? cc @tim-mwangi @ryanericson ?

[varkey98]

Since this is the key input from gateway agents, I think it should be prefixed with traceableai. I'd say traceableai.environment.name is a better key

[thugrock7]

Wanted to keep "traceable" name out of hypertrace repos. Don't know the convention we follow though

[varkey98]

I'd rather keep environment notion itself out of this repo. It'll be scattered if you add that notion here. Because initially we made it traceable specific one. ref, https://github.com/Traceableai/agent-config/blob/main/proto/ai/traceable/agent/config/v1/config.proto#L22

[varkey98]

Now it's gonna be like hypertrace agent provides environment specific tracers, but traceable goagent is used for getting environment specific filters when you think about it from a tracer-filter pair pov

[thugrock7]

I agree to keep environment notion out of this repo. Just thinking on lines of making traceable-goagent to have multi environment support as the TA, to register services there we mitigate that logic to RegisterServiceWithSpanProcessorWrapper() (here), the only problem is we assume a unique key for traceProviders here which service name used to do. But if we want to create a multi-env support for traceable-goagent, where a same service name can exist in different environments, this current setup fails due to this traceProviderMap. So here I only intend to use unique key to create tracer.
May be other way is to form a unique key in traceable-goagent itself, and use that to call the RegisterServiceWithSpanProcessorWrapper(unique_key, resourceAttributes), but as here we add resource attrs, didn't feel it generic enough and dropped that idea

[varkey98]

I still dont understand why you can't add all of this logic in TPA itself?

[varkey98]

Is it that the argument name is service_name across our api, but actually we would be passing environment::sep::service_name?

[thugrock7]

I still dont understand why you can't add all of this logic in TPA itself?

Currently we have for each service in extcap a unique tracerfilterpair .The tracerSets of ext_cap(link) use goagent to create a tracer, which in current setup will fail for cases of same service name which can be case in multiple environments. So to avoid depending on only service name as key which can no longer be unique, formed the unique key here (encode) itself in PR. Although this could could come from TPA itself, then we should decode for place like here to get to add resource attrs, and get service name etc correctly.

[varkey98]

why do you want to decode the value? Afair, the map itself would be coming from TPA right?

[varkey98]

Another thing is that all of these api's written assuming that service name is a resource attribute. The moment we start piggy backing on them for treating environment similarly to service name there'll be a disparity as TPA processors assume them to be a span attribute. This will happen across agents and easiest thing would be to use attributeaction processor to put this under both span and resource attributes till we update processors

[tim-mwangi]

Hey @thugrock7, what is this needed for? Do we want to support multiple environments in tracers as well?

[thugrock7]

I was trying to make multi-environment support for ext_cap of TPA, which followed to make traceable-goagent to support multiple environments. It's different to other TA like ebpf, where TA can only one have environment. cc: @tim-mwangi