feat: expand fsm instrumentation #2177
Conversation
jonathanj-square
requested review from
a team and
deniseli
and removed request for
a team
…rmine whether the upsert operation is an update or an insert. continued fsm instrumentation
jonathanj-square
force-pushed
the
jonathanj/otel/fsm2
branch
from
3e4e23d to
5aafee3
Compare
| @@ -0,0 +1,6 @@ | |||
| -- migrate:up | |||
|
|
|||
| ALTER TABLE fsm_instances | |||
backend/controller/dal/fsm_test.go
Outdated
| @@ -15,6 +16,8 @@ import ( | |||
| ) | |||
|
|
|||
| func TestSendFSMEvent(t *testing.T) { | |||
| _ = observability.InitControllerObservability() | |||
There was a problem hiding this comment.
I wasn't sure it was a good idea, but I think that having to do this everywhere is a good argument for statically initialising observability at init() time.
There was a problem hiding this comment.
Yeah totally agree. That would not be fun to have to remember this for every test that might have a metric. init() sounds good to me. Will probably clean up quite a bit of this plumbing too.
There was a problem hiding this comment.
feedback adopted
| func FSM() *FSMMetrics { | ||
| return fsm | ||
| } |
There was a problem hiding this comment.
We can probably just expose the public var here vs. having a wrapper func. This is likely more true with an init() approach.
There was a problem hiding this comment.
feedback adopted
| if result.instancesActive, err = result.meter.Int64UpDownCounter( | ||
| counter, | ||
| metric.WithDescription("counts the number of active FSM instances")); err != nil { | ||
| errs = errors.Join(errs, fmt.Errorf("%q counter init failed; falling back to noop: %w", counter, err)) |
There was a problem hiding this comment.
Just to dedup the repeated code, what do you think about something like:
errs = joinInitErrors(errs, err, counter)
...
}
func joinInitErrors(...) error {
return errors.Join(errs, fmt.Errorf("%q counter init failed; falling back to noop: %w", counter, err))
}
| fsmCounters.instancesActive, err = fsmMeter.Int64UpDownCounter( | ||
| fmt.Sprintf("%s.instances.active", fsmMeterName), | ||
| metric.WithDescription("counts the number of active FSM instances")) | ||
| result.meter = otel.Meter("ftl.fsm") |
There was a problem hiding this comment.
result.meter = otel.Meter(fsmMeterName)
fsm_instancestable scheme to help distinguish between insert and update.Issue: #2194