Add support for Resources in the SDK #552
Conversation
Add `Config` types for the push `Controller` and the `SDK`. Included with this are helper functions to configure the `ErrorHandler` and `Resource`. Add a `Resource` to the Meter `Descriptor`. The choice to add the `Resource` here (instead of say a `Record` or the `Instrument` itself) was motivated by the definition of the `Descriptor` as the way to uniquely describe a metric instrument. Update the push `Controller` and default `SDK` to pass down their configured `Resource` from instantiation to the metric instruments.
MrAlias
requested review from
jmacd,
krnowak,
lizthegrey,
paivagustavo,
rghetia and
tedsuo
as code owners
How about WithResource as an option? I guess same could be applied to unit, description, and labels as well. |
I think this is a good direction. It would follow our other design patterns as well. I'll refactor. |
Add DescriptorConfig and DescriptorOption to configure the metric Descriptor with the description, unit, keys, and resource. Update all function calls to NewDescriptor to use new function signature.
Co-Authored-By: Rahul Patel <[email protected]>
|
I have a selfish favor to ask. Would you mind waiting until #560 merges? (If it merges.) This PR will cause a lot of conflicts with #560, which I'm nearly finished with. #560 was a really difficult refactoring, and I'm worried about conflicts. The cause of the conflict is that I needed a Descriptor type in the API to organize the two new constructors (NewSynchronousInstrument, ...), so I removed I am self-reviewing #560 now and adding comments and cleanups, but it's building and passing tests. |
Yeah, no problem. |
|
I think that the discussion about option as interface vs function can be moved to #536, so no need to block this PR on it. Still there was one issue about forwarding the error handler to address. |
Pass the configured ErrorHandler for the controller to the SDK.
Add back the Resource field to the Descriptor that was moved in the last merge with master. Add a resource.Provider interface. Have the default SDK implement the new resource.Provider interface and integrate the new interface into the newSync/newAsync workflows. Now, if the SDK has a Resource defined it will be passed to all Descriptors created for the instruments it creates.
Add an `Equal` method to the Resource so it can be compared with github.com/google/go-cmp/cmp. Add additional test of the API Option unit tests to ensure WithResource correctly sets a new resource.
Move the interface to where it is used. Fix spelling.
Co-Authored-By: Rahul Patel <[email protected]>
Add
Configtypes for the pushControllerand theSDK. Included with this are helper functions to configure theErrorHandlerandResource.Add a
Resourceto theDescriptor. The choice to add theResourcehere (instead of say aRecordor theInstrumentitself) was motivated by the definition of theDescriptoras the way touniquely describe a metric instrument.
Update the push
Controllerand defaultSDKto pass down their configuredResourcefrom instantiation to the metric instruments.Resolves #515