Skip to content
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

refactor(ipld): migrate metrics scheme #1207

Merged
merged 11 commits into from
Nov 28, 2024
Merged

Conversation

LePremierHomme
Copy link
Contributor

Closes #1093

Current work contains the migration to the event-based architecture.
Still need to evaluate the usefulness of each metric.

@LePremierHomme LePremierHomme requested a review from a team as a code owner November 19, 2024 11:09
@karlem
Copy link
Contributor

karlem commented Nov 19, 2024

Thanks @LePremierHomme! I took a quick look, and overall it looks good. I may have more comments tomorrow. We will also need to add these events to the documentation and ensure that the traces produce user-friendly output.

@LePremierHomme
Copy link
Contributor Author

I added domain namespace prefix to the traces, although it's not applied elsewhere.
So it looks like this now:

2024-11-20T05:13:52.295725Z  INFO tracing_event: domain="IPLD_Resolver/Ping" event=Success(PeerId("1AkHsDuvAgj8ZAAgGaaqw82CMRU6e7ii2WA4XmXDsj1qkQ"), 500ms)
2024-11-20T05:13:52.296359Z  WARN tracing_event: domain="IPLD_Resolver/Ping" event=Timeout(PeerId("1AkHsDuvAgj8ZAAgGaaqw82CMRU6e7ii2WA4XmXDsj1qkQ"))
2024-11-20T05:13:52.296402Z  WARN tracing_event: domain="IPLD_Resolver/Ping" event=Failure(PeerId("1AkHsDuvAgj8ZAAgGaaqw82CMRU6e7ii2WA4XmXDsj1qkQ"), "err")

As for the docs, I'll add it as the final step once we're done with the review.

ipld/resolver/src/observe.rs Show resolved Hide resolved
ipld/resolver/src/observe.rs Outdated Show resolved Hide resolved
@LePremierHomme LePremierHomme merged commit 7572a6a into main Nov 28, 2024
15 checks passed
@LePremierHomme LePremierHomme deleted the migrate-ipld-metrics branch November 28, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Migrate Resolver Metrics to New Observability Event-Based Architecture
2 participants