Open ptravers opened 4 years ago
Thanks for opening this issue! I was already exploring what would be involved in making Money an implementation of OpenTelemetry by either implementing the OpenTelemetry API interfaces directly or by providing a wrapper type (probably the latter, less disruptive). I've got a small list of changes to Money that I think would be necessary to support the additional functionality of OpenTelemetry. I can post them later today or tomorrow.
Changes to spans:
Note[T]
types to include String, Long, Double and Boolean arrays.Note[T]
There is also a bit more state in an OTel Span that we might need to add:
tracestate
header)Some of this could probably be stored as notes but it might get a bit noisy.
I am still digging around in the OTel APIs and may suss out a few more things. @ptravers If you know of anything I'm missing please let me know.
I think this makes sense. Adding methods to the API are good.
I wonder if money can directly implement any open-telemetry API / interface(s) as well? In other words, abandon what money has in it's own java interfaces, and substitute with what is in open-telemetry, updating the money-core as needed. Granite I have not looked at all at any of this stuff so this is spitballing a bit.
I wonder if money can directly implement any open-telemetry API / interface(s) as well? In other words, abandon what money has in it's own java interfaces, and substitute with what is in open-telemetry, updating the money-core as needed. Granite I have not looked at all at any of this stuff so this is spitballing a bit.
Certainly possible. There's a lot of overlap between the interfaces and with the exception of the handful of concepts that Money doesn't (yet) support that I mentioned above the members do more or less map one-to-one with Money APIs. That may be a bit confusing to users, though, if they see both end
and stop
methods for stopping spans, and record
and addAttribute
methods for adding notes to spans. Not to mention the name collisions between Tracer
, Span
and maybe a few others. But perhaps these are minor issues, or perhaps we move away from the Money APIs and towards the OTel APIs instead?
I would be more in favour of having a breaking release and updating the money interface to match OTel. I think the main issue we will have is making sure people can continue to get the data as structured logs and that the log structure doesn't change. We don't want to break everyone's dashboards.
We should though have a release where we deprecate stop
and add end
I can start on a WIP PR that has the Money interfaces extend from the OTel interfaces and see where that lands.
More spaghetti at the wall, maybe we can keep the Money members intact but deprecated as a first cut release and then remove those members entirely in a second release?
Yeah definitely! Sorry that's what I was trying to say poorly in my comments
Thank you @HaloFour for stepping up and picking up this work
The major work to wrap the Money API in an OpenTelemetry API is complete. I have a few additional PRs up to improve the integration:
I'm also looking at changes to propagate trace flags (sampled) and trace state through the Money SpanId/SpanContext so that Money can propagate them between upstream and downstream systems. This will also impact Money Formatters which will have to convey that state. I was considering an adapter between Money Formatters and OTel TextMapPropagators so that we can use off-the-shelf propagators rather than maintaining our own. OTel already has implementations for ZipKin and TraceContext, as well as a few others. We could even make formatters configurable like handers are.
Once SpanId can track sampling flags adding a configurable sampler to Money for new traces should be trivial and SpanHandlers can be written to respect those flags.
I believe I've completed everything I mentioned above:
money-otel-formatters
):
SpanId
of the remote span and all child spans so that it can be interpreted in any SpanHandler as well as propagated upstreammoney-otel-zipkin-exporter
)money-otel-jaeger-exporer
)Possible additional projects:
I'm creating another branch in which I plan on following the snapshot releases of OpenTelemetry API 0.10.0 and working on any of the breaking changes. So far it doesn't seem too bad. A few things renamed, a few things moved around. The biggest changes are around span context as the GRPC Context has been replaced with their own implementation. A lot of the Span methods also now return the Span so I need to wrap them in the covariant interface and return the instance.
I created this issue to have conversation over what we want to do with Money. To be more specific, the expected output of this issue is hopefully a clear direction as to what the library should become.
My personal view is that we should help our users migrate to Open Telemetry