-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
otel: change the go-runtime SDK to use a constant string (or configurable?) #2026
Comments
@deniseli maybe we should just remove this file for now since it's not being used. We can re-introduce something we're more happy with at that time? I think we're also using the verb name for |
@wesbillman yeah agreed, getting rid of the file for now seems like the right call. |
Part 1 of #2026 This deletes the whole observability package from the Go SDK, since no one is using it yet. We'll replace it with an interface that better aligns with our internal otel usage patterns once that's more mature. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Current thoughts: it'd be nice to make the go-runtime SDK observability interface more declarative than what otel forces you into. We can simply wrap otel for now and give the users better functions for interfacing with it. For any niche features that we can't support, there's always the backdoor of invoking otel directly in the user code. Design Thoughts Part 1: AttributesIt'd be nice to pass attributes via struct (so the types are built-in) instead directly using the Suppose you're a user, and one of your verbs calls out to a somewhat flaky external API. You want to instrument some metrics on your verb with attributes recording the details of that API call:
The Go SDK could use
|
Design Thoughts Part 2: Metric instantiationMeters and metrics don't have to be instantiated at initialization time. In fact, the user code will be cleaner if they don't need to think about that. The SDK could just provide a set of functions to create and manage metrics:
To support all the main signal types (Counter, Histogram, and Gauge), we'll need to flesh out this interface with some more functions. Options:
Note: Leaving UpDownCounter out for now because if any up or down update is missed, then the observed value would be over or under, which could lead to confusion. Safer to funnel users into using Histo or Gauge. The meter itself could be instantiated and held as a singleton at the package scope. |
note from standup: this should also build in constraints on attribute cardinality since we get charged for that by datadog |
THIS IS LOOOOWWWW PRIORITY! (because it's both not in use and opt-in)
In go-runtime/ftl/observability/metrics.go, we use the verb name as the metric name, which would result in a seeing a quite high number of metrics. This is not ideal; we don't want FTL to have a reputation for being expensive to instrument logging for.
As an alternative, users are free to invoke the
otel
SDK directly in their code, and the globalotel
configuration/setup we've done will apply.The text was updated successfully, but these errors were encountered: