Move Resource to SDK #86
Conversation
fbogsany
requested review from
bai,
luvtechno,
mwear and
SergeyKanzhelev
as code owners
| @@ -9,6 +9,8 @@ module SDK | |||
| module Trace | |||
| # {Tracer} is the SDK implementation of {OpenTelemetry::Trace::Tracer}. | |||
| class Tracer | |||
| attr_accessor :resource # TODO: should this be read-only? If not, how should exporters know when it is updated? | |||
There was a problem hiding this comment.
In the java SDK the resource is final: https://github.com/open-telemetry/opentelemetry-java/blob/master/sdk/src/main/java/io/opentelemetry/sdk/trace/TracerSdk.java#L50 and assigned as an EnvVarResource, which we don't yet have. We could consider doing the same and turn this into a reader.
There was a problem hiding this comment.
Yeah, I wasn't sure how we wanted to configure the Resource. Go has WithService and WithResource factory methods for the Tracer, but they're not wired up to anything yet. I'd like to tackle configuration of the Resource in a subsequent PR.
There was a problem hiding this comment.
The WithResource/WithService should be moved on the TracerSDK. Only Component will be available in the API based on the OTEP-007 (better than 0007)
There was a problem hiding this comment.
OTEP-007 (better than 0007)
🤣
There was a problem hiding this comment.
@bogdandrutu this PR moves the Resource to the SDK, but I didn't see anything in the OTEP-007 indicating how the Resource should be configured. Is the plan to only support configuration from environment variables, as in the Java SDK, or should we also allow explicit manual configuration like the Go SDK?
|
Open #99 to discuss the Resource initialization concern. Okay if we merge this and discuss in that issue? |
Fixes #83