-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
dfe659a
to
570ded3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One blocker about metrics
endpoint being public. Otherwise looks good. Left some questions and suggestions.
Thank you for replacing the summary endpoint 👍
eventrecorder/recorder.go
Outdated
handleQueryAskEvent() | ||
case types.QueryAskedFilteredCode: | ||
handleQueryAskFilteredEvent() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log warning on default case. This would be useful to detect missing cases when lassie types are updated but this code is not.
redesign metrics to target opentelemetry, handle events in intelligent ways, and produce usable lassie metrics
Co-authored-by: Masih H. Derkani <m@derkani.org>
Co-authored-by: Masih H. Derkani <m@derkani.org>
atomic.StoreUint32(&t.failedCount, 0) | ||
} | ||
|
||
var tempDataPool = sync.Pool{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gather that we're just relying on the sheer volume of traffic to wipe out the errors introduced by GC wiping away our data occasionally, giving us not-quite-accurate finality reports? With a very busy service, I wonder whether GC is going to get really disruptive here and make this very inaccurate?
also adds setup for docker compose
Goals
Put some metrics into the event recorder so we can verify that we've got a prometheus flow moving end to end.
Implementation
For now I just took the metrics recorder from Lassie directly and did a best effort translation to try to record some metrics. Some of these have a few deficiencies but it's all going to change once we ship filecoin-project/lassie#138
For discussion
Remove that summary endpoint, cause it never worked and was a terrible idea