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

feat: add tracing, request timeouts and move instrumentation to boxo #87

Merged
merged 16 commits into from
Nov 19, 2024

Conversation

2color
Copy link
Member

@2color 2color commented Nov 13, 2024

What's in this PR

  • Remove instrumentation from server
  • Add accept header to logging
  • Add tracing with sampling or require token for requests with Traceparent header
  • Add 30 second request timeout

Why

TL;DR: expose metrics per handler/route and use more

See ipfs/boxo#718.

Depends on

@2color 2color changed the title feat: move instrumentation to boxo feat: add tracing, request timeouts and move instrumentation to boxo Nov 13, 2024
@2color 2color marked this pull request as ready for review November 14, 2024 08:29
this isn't working too well. We need to look more deeply into this
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged boxo PRs and switched this one to a commit from boxo main branch with both.

I think we should ship this do delegated-routing.dev, try to visualize metrics in grafana, nd see if bucket sizes make sense (duration should be ok, but sizes may be too big, we won't have 1GB response from routing system, IPNS record size is capped at 10KiB (https://specs.ipfs.tech/ipns/ipns-record/#record-size-limit)

@2color Feel free to merge this PR and then open one that releases this as v0.6.0

tracing.go Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
Co-authored-by: Marcin Rataj <lidel@lidel.org>
@2color
Copy link
Member Author

2color commented Nov 19, 2024

@lidel See ipfs/boxo#724 for updated size buckets

@2color 2color merged commit c4859ec into main Nov 19, 2024
7 checks passed
@2color 2color deleted the improve-metrics branch November 19, 2024 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants