Feature/keystone beholder integration #14510
Conversation
core/logger/logger.go
Outdated
| @@ -197,6 +197,7 @@ func (c *Config) New() (Logger, func() error) { | |||
|
|
|||
| l = newSentryLogger(l) | |||
| l = newPrometheusLogger(l) | |||
| // l = newBeholderLogger(l) | |||
There was a problem hiding this comment.
could hook in as early as here but seems excessive (all overloaded funcs will emit a custom message) for this first use case of integrating with Keystone
core/logger/beholder.go
Outdated
| ) | ||
|
|
||
| // beholderCustomMessageLogger is used to emit logs as custom messages through Beholder | ||
| type beholderCustomMessageLogger struct { |
There was a problem hiding this comment.
ordinarily if you want to log, you would just log.
However, we will not have access to metrics through Beholder to start so this wrapper is necessary.
ST3 currently requires log messages to be surfaced through Beholder. This allows the o11y consumer to decide which Loki source to ship the logs to, potentially unblocking external viewership.
There was a problem hiding this comment.
🤔 Is this needed if we go with option A? IIUC it isn't, correct?
There was a problem hiding this comment.
That's correct - if we go with option A we don't need the wrapped logger
patrickhuie19
changed the title
core/services/workflows/engine.go
Outdated
| // TODO ks-461 | ||
|
|
||
| // ks-461 Option A: | ||
| logAndSendCustomMessage(e.logger, "initialization failed: %s", retryErr) |
There was a problem hiding this comment.
I slightly prefer this option: this doesn't couple our logger + custom message implementation together so we can flexible diverge the two (logs + custom messages). It'll make things more verbose, but also more explicit.
There was a problem hiding this comment.
The tricky part with A is getting access to all the sugared values that have been added to the logger. Maybe the best path forward is to look for specific values in the drilled context? For tracing those labels will be valuable as well if added to the context.
patrickhuie19
force-pushed
the
feature/keystone-beholder-alerts
branch
from
1f8a73d
to
25800ef
Compare
|
I see you updated files related to
|
|
Supports https://chainlink-core.slack.com/archives/C07EPTPTA74/p1726839132067279