chore: Update logging to use layr-labs/eigensdk-go
#264
Conversation
ethenotethan
changed the title
layr-labs/eigengo-sdk
ethenotethan
changed the title
layr-labs/eigengo-sdklayr-labs/eigensdk-go
… changes, deprecate flags, and update docs / code comments
common/logging_cli.go
Outdated
| Name: common.PrefixFlag(flagPrefix, FormatFlagName), | ||
| Category: category, | ||
| Usage: "The format of the log file. Accepted options are 'json' and 'text'", | ||
| Value: "json", |
There was a problem hiding this comment.
can update - but this is the default for eigenda
There was a problem hiding this comment.
Does it matter? Very unclear when we'll actually merge the two. And in the meantime we'll have to read these logs.
There was a problem hiding this comment.
Also this is a breaking change for users of this right? They might have had infra setup to ingest our previous log formats. Were those text?
There was a problem hiding this comment.
lemme evaluate - if both op-log and eigensdk-go/logger use the same slogger under the hood then it should be ok. I flagged this as a risk in the initial PR description.
There was a problem hiding this comment.
yeah output formats change: https://www.diffchecker.com/hFA97XKJ/
Let's just make sure to be very loud in our comms about this potential observability risk?
There was a problem hiding this comment.
Yeah its not that bad. still a golang format key-value pair output, so should be fine.
Something looks off though. Some lines use different logging format than others.
There was a problem hiding this comment.
Just realized this is potentially a breaking change for customers right? See #264 (comment)
… e and copy entire dependency package vs hyrbid
… patterns across subpackages to adhere to uniform abstraction
…sk--chore-update-logger-eigen-sdk
… patterns across subpackages to adhere to uniform abstraction
Fixes Issue
Fixes #
Changes proposed
Updates all logging references to use
eigengo-sdk/logging. This ensures better compatibility with the mono repo. Also refactored our metrics to no longer be an OP dependency and instead a local one 😄More specifically:
Key Consideration
The slogger leveraged here dumps out json format logs. This could potentially bork existing loki / observability integrations that our customers use for viewing proxy output. We will need to articulate this very clearly in our release notes to ensure our customers have appropriate visibility.
Blocked by
Screenshots (Optional)
Note to reviewers