OpenTelemetry: metrics for ActivityPub collections#777
Conversation
Add Fedify collection metric instruments and helper functions so collection request, dispatcher duration, page item count, and total item count measurements share a bounded attribute set. fedify-dev#316 fedify-dev#741 Assisted-by: Codex:gpt-5.5
Wire the collection metric instruments into built-in and custom collection request handling so operators can observe request outcomes, dispatcher latency, item counts, and reported total item counts without recording collection identifiers or cursors. fedify-dev#316 fedify-dev#741 Assisted-by: Codex:gpt-5.5
Document the new ActivityPub collection metrics and changelog entry so operators can discover the request, dispatch latency, item count, and total item count measurements and their bounded attributes. fedify-dev#316 fedify-dev#741 fedify-dev#777 Assisted-by: Codex:gpt-5.5
dahlia
added
component/federation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds bounded OpenTelemetry metrics for ActivityPub collection handling (request counter, dispatch-duration histogram, page-items and total-items histograms), instruments built-in and custom collection handlers and middleware, adds unit/integration tests, and updates docs and changelog. ChangesCollection request metrics instrumentation
Sequence DiagramsequenceDiagram
participant Client
participant Middleware
participant Handler
participant FederationMetrics
Client->>Middleware: HTTP GET collection (cursor?)
Middleware->>Handler: route to built-in/custom handler (meterProvider?)
Handler->>FederationMetrics: buffer dispatch duration & item counts
Handler-->>Middleware: respond (served/not_found/error)
Middleware->>FederationMetrics: recordCollectionRequest(result, page, status)
Handler->>FederationMetrics: flush buffered metrics with final result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
dahlia
moved this to In review
in STF 2025–2026 Milestone 6: Observability and Monitoring (OpenTelemetry)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements OpenTelemetry metrics for ActivityPub collection requests, including counters for requests and histograms for dispatch duration, page items, and total items. These metrics are integrated into both built-in and custom collection handlers, with corresponding updates to documentation and test suites. Review feedback suggests optimizing the getCollectionMetricKind function by using a Set for membership checks and avoiding redundant string normalization to improve efficiency and maintainability.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b843c6a0bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/fedify/src/federation/handler.ts`:
- Line 471: The variable baseUri returned by uriGetter(identifier) is declared
only inside the if (cursor == null) block but later referenced in the
paginated/else branch, causing a build error; fix by moving the baseUri
computation out of the conditional so it’s in scope for both branches (e.g.,
compute const baseUri = uriGetter(identifier) before the if (cursor == null)
check or recompute it in the else), and update any subsequent uses in the
paginated branch that reference baseUri to use that shared variable; relevant
symbols to locate are baseUri, uriGetter, identifier and the cursor conditional
in handler.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb2702d5-9f7d-4e0c-866f-9f15ea2199c6
📒 Files selected for processing (8)
CHANGES.mddocs/manual/opentelemetry.mdpackages/fedify/src/federation/handler.test.tspackages/fedify/src/federation/handler.tspackages/fedify/src/federation/metrics.test.tspackages/fedify/src/federation/metrics.tspackages/fedify/src/federation/middleware.test.tspackages/fedify/src/federation/middleware.ts
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
Record custom collection dispatcher measurements only after the handler knows the final request result, so serialization failures classify the duration, item count, and total item count histograms as errors instead of served responses. Also normalize built-in collection metric names once through a shared set. fedify-dev#777 (comment) fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fba894f2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry metrics for ActivityPub collection requests, including counters for request volume and histograms for dispatch duration, page item counts, and total item counts. The changes span the federation handlers, middleware, and documentation, ensuring that both built-in and custom collections are instrumented with bounded cardinality. A medium-severity issue was identified in the normalization logic for collection kinds, where camelCase route names like featuredTags are not correctly converted to snake_case, leading to incorrect classification in the metrics.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/fedify/src/federation/handler.ts`:
- Around line 2111-2116: The metrics currently push itemCount using
page.items.length which is measured before filtering and thus can overreport;
update the push to record the length of the filtered/serialized items returned
by getPageProps() or getPropsWithoutCursor() (i.e. use the array produced after
filterItems() runs) so pendingCollectionMetrics.itemCount reflects the actual
activitypub.collection.page.items sent to clients; locate the push near
pageMetricBase and replace page.items.length with the filtered result length
returned by those methods.
- Around line 2166-2190: The queued metrics drop http.response.status_code
because recordPendingCollectionMetrics rebuilds attributes only from
metricAttributes(measurement.page, result); fix by preserving and passing the
original response/status from the queued measurement into attribute rebuilding:
update recordPendingCollectionMetrics to call metricAttributes with the
measurement's response/status (e.g., metricAttributes(measurement.page, result,
measurement.response) or measurement.responseStatusCode), and if necessary
extend metricAttributes' signature and the code that enqueues into
`#pendingCollectionMetrics` so each measurement includes the original response or
status code; this ensures recordCollectionDispatchDuration,
recordCollectionPageItems and recordCollectionTotalItems receive attributes
including http.response.status_code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3d72130-2670-420f-a73b-5f3de919d3ee
📒 Files selected for processing (2)
packages/fedify/src/federation/handler.test.tspackages/fedify/src/federation/handler.ts
Keep custom collection histogram attributes aligned with the final response status, including served and not_found responses. Record page item counts after fallback filtering so metrics match the collection payload Fedify serializes, and normalize camelCase collection names when classifying built-in collection metric kinds. fedify-dev#777 (comment) fedify-dev#777 (comment) fedify-dev#777 (comment) fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dec47bad87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/fedify/src/federation/handler.ts (2)
341-343: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDocument the new public
meterProvideroption.Both exported handler parameter interfaces gained a public telemetry hook, but the new field is undocumented. Please add a short JSDoc block like the one on
InboxHandlerParameters.meterProviderso it shows up in the public API docs.As per coding guidelines,
packages/*/src/**/*.{ts,tsx}: Include JSDoc comments for all public APIs.Also applies to: 1713-1715
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/fedify/src/federation/handler.ts` around lines 341 - 343, The new public meterProvider option in the handler parameter interfaces is missing JSDoc; add a short JSDoc comment above the meterProvider property (matching the style used for InboxHandlerParameters.meterProvider) so it appears in the public API docs—update both places where meterProvider is declared alongside tracerProvider and onUnauthorized (referencing the TracerProvider and MeterProvider types and the onUnauthorized method signature) and ensure the comment briefly describes its purpose as a telemetry hook and expected type.
513-515:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the filtered item count for built-in collection metrics.
handleCollection()still recordsitemCountfrom the raw dispatcher result beforefilterCollectionItems()runs, soactivitypub.collection.page.itemswill overreport whenever fallbackfilterPredicateremoves items. The custom path was fixed for this exact mismatch; the built-in path needs the same treatment, plus a regression test coveringhandleCollection()withfilterPredicate.Also applies to: 537-537, 585-586, 622-622
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/fedify/src/federation/handler.ts`:
- Around line 341-343: The new public meterProvider option in the handler
parameter interfaces is missing JSDoc; add a short JSDoc comment above the
meterProvider property (matching the style used for
InboxHandlerParameters.meterProvider) so it appears in the public API
docs—update both places where meterProvider is declared alongside tracerProvider
and onUnauthorized (referencing the TracerProvider and MeterProvider types and
the onUnauthorized method signature) and ensure the comment briefly describes
its purpose as a telemetry hook and expected type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: baa76040-32dc-4d0c-b032-a108c2825ec7
📒 Files selected for processing (2)
packages/fedify/src/federation/handler.test.tspackages/fedify/src/federation/handler.ts
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry metrics for ActivityPub collection requests, including counters for requests and histograms for dispatch duration, page items, and total items. The changes span federation handlers, middleware, and documentation. Feedback indicates that the itemCount metric should be updated to reflect the filtered number of items actually served in the response—rather than the raw count from the dispatcher—to ensure the metrics provide accurate operational insights.
Add public API documentation for the collection meterProvider options so the telemetry hook appears alongside the existing handler parameter docs. Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91acb27db8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/fedify/src/federation/handler.ts (2)
589-626:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame pre-filtered item count issue in paginated branch.
Apply the same fix as the non-paginated case: compute filtered items first, then set
itemCountfrom the filtered result.🛡️ Proposed fix
const { items, prevCursor, nextCursor } = pageOrResponse; + const filteredItems = filterCollectionItems(items, name, filterPredicate); + itemCount = filteredItems.length; let prev = null; if (prevCursor != null) { @@ collection = new OrderedCollectionPage({ id: uri, prev, next, - items: filterCollectionItems(items, name, filterPredicate), + items: filteredItems, partOf, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/fedify/src/federation/handler.ts` around lines 589 - 626, The paginated branch sets itemCount from page.items before filtering, causing incorrect counts; update the logic around pageOrResponse handling (the code that builds OrderedCollectionPage and uses filterCollectionItems) to first compute filteredItems = filterCollectionItems(items, name, filterPredicate), then set itemCount = filteredItems.length (and set span attribute "fedify.collection.items" from that filtered count) before constructing the OrderedCollectionPage with items: filteredItems; ensure the same variables (pageOrResponse, items, prevCursor, nextCursor, OrderedCollectionPage, filterCollectionItems, and itemCount/span.setAttribute) are updated accordingly.
517-541:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBuilt-in handler records pre-filtered item count, inconsistent with custom handlers.
itemCountis set toitems.length(line 518) beforefilterCollectionItemsruns (line 541). Custom handlers record the post-filtered count (lines 2065-2066, 2109-2110). WhenfilterPredicateremoves items, the built-in metric overreports.🛡️ Proposed fix to use filtered count
if (itemsOrResponse instanceof Response) { return finish(itemsOrResponse, "not_found"); } + const filteredItems = filterCollectionItems(itemsOrResponse, name, filterPredicate); + itemCount = filteredItems.length; collection = new OrderedCollection({ id: baseUri, totalItems: totalItemCount ?? null, - items: filterCollectionItems(itemsOrResponse, name, filterPredicate), + items: filteredItems, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/fedify/src/federation/handler.ts` around lines 517 - 541, The built-in handler records itemCount from the unfiltered page items (variable itemCount = items.length) before filterCollectionItems is applied, causing metrics to overreport when filterPredicate removes items; fix by computing itemCount after filtering by replacing the pre-filter assignment with setting itemCount = filteredItems.length (or equivalent) using the result of filterCollectionItems (referencing filterCollectionItems, itemsOrResponse, itemCount, and the OrderedCollection construction) so both built-in and custom handlers record the post-filtered count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/fedify/src/federation/handler.ts`:
- Around line 589-626: The paginated branch sets itemCount from page.items
before filtering, causing incorrect counts; update the logic around
pageOrResponse handling (the code that builds OrderedCollectionPage and uses
filterCollectionItems) to first compute filteredItems =
filterCollectionItems(items, name, filterPredicate), then set itemCount =
filteredItems.length (and set span attribute "fedify.collection.items" from that
filtered count) before constructing the OrderedCollectionPage with items:
filteredItems; ensure the same variables (pageOrResponse, items, prevCursor,
nextCursor, OrderedCollectionPage, filterCollectionItems, and
itemCount/span.setAttribute) are updated accordingly.
- Around line 517-541: The built-in handler records itemCount from the
unfiltered page items (variable itemCount = items.length) before
filterCollectionItems is applied, causing metrics to overreport when
filterPredicate removes items; fix by computing itemCount after filtering by
replacing the pre-filter assignment with setting itemCount =
filteredItems.length (or equivalent) using the result of filterCollectionItems
(referencing filterCollectionItems, itemsOrResponse, itemCount, and the
OrderedCollection construction) so both built-in and custom handlers record the
post-filtered count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96f306fa-d769-4045-8ebb-61849e673fe4
📒 Files selected for processing (1)
packages/fedify/src/federation/handler.ts
There was a problem hiding this comment.
Code Review
This pull request implements OpenTelemetry metrics for ActivityPub collection requests, introducing counters for request volume and histograms for dispatch duration, page items, and total items. The changes affect federation handlers, middleware, and documentation, providing instrumentation for both built-in and custom collection dispatchers. Review feedback identifies a style guide violation in the changelog and a potential concurrency risk associated with using a global WeakMap for metric recorders, recommending that error objects be uniquely instantiated per request.
Record built-in collection item metrics after fallback filtering so activitypub.collection.page.items matches the response payload served to clients. Add regression coverage for both whole collection and paginated collection responses. fedify-dev#777 (comment) fedify-dev#777 (comment) fedify-dev#777 (comment) fedify-dev#777 (comment) fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/fedify/src/federation/handler.ts (1)
2189-2193: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMinor: use
pageMetricBase.pagefor consistency.Line 2191 re-computes
cursor !== nullinstead of using the already-computedpageMetricBase.pagefrom line 2174. Functionally equivalent, but using the same source improves readability.🔧 Suggested change
} catch (e) { this.#pendingCollectionMetrics.push({ - page: cursor !== null, + page: pageMetricBase.page, dispatchDurationMs: getDurationMs(started), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/fedify/src/federation/handler.ts` around lines 2189 - 2193, Replace the re-computation of the page flag in the catch block with the already-computed value: instead of pushing page: cursor !== null into this.#pendingCollectionMetrics, use pageMetricBase.page (the value computed earlier). Update the push into this.#pendingCollectionMetrics to reference pageMetricBase.page while keeping dispatchDurationMs computed via getDurationMs(started).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/fedify/src/federation/handler.test.ts`:
- Around line 1289-1334: Test "handleCollection() records filtered collection
item metrics" only asserts metric value; update the test to also assert the
measurement.attributes (e.g., check attributes like activitypub.collection.page,
activitypub.collection.result, and status) for the measurement returned by
recorder.getMeasurements("activitypub.collection.page.items") so the page path
classification and result/status metadata are validated; locate the assertions
after the response.status check and add checks that items[0].attributes includes
the expected page/path and result/status values. Apply the same hardening to the
sibling test block around the later case (the other filtered-metrics test
covering lines ~1336-1382) using the same functions: handleCollection,
recorder.getMeasurements, filterPredicate and the dispatcher-based
collectionCallbacks.
---
Outside diff comments:
In `@packages/fedify/src/federation/handler.ts`:
- Around line 2189-2193: Replace the re-computation of the page flag in the
catch block with the already-computed value: instead of pushing page: cursor !==
null into this.#pendingCollectionMetrics, use pageMetricBase.page (the value
computed earlier). Update the push into this.#pendingCollectionMetrics to
reference pageMetricBase.page while keeping dispatchDurationMs computed via
getDurationMs(started).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 163be933-358e-4cbb-bed9-420dffc26e0e
📒 Files selected for processing (2)
packages/fedify/src/federation/handler.test.tspackages/fedify/src/federation/handler.ts
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request implements OpenTelemetry metrics for ActivityPub collection requests, adding counters for request volume and histograms for dispatch latency, item counts per page, and total collection items. The changes span the core federation handlers, metric definitions, and documentation, ensuring bounded cardinality for metric dimensions. Review feedback identifies potential type mismatches where totalItems (potentially a bigint) is recorded; it is recommended to explicitly cast these values to number for OpenTelemetry compatibility.
The review caught that the filtered item-count regression tests only checked the recorded value. Assert the key collection metric attributes as well so the tests also cover page classification, served result, and HTTP status metadata. Addresses: fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request implements OpenTelemetry metrics for ActivityPub collection requests, covering both built-in and custom collection handlers. New metrics include activitypub.collection.request, activitypub.collection.dispatch.duration, activitypub.collection.page.items, and activitypub.collection.total_items, with dimensions designed to maintain bounded cardinality. The changes include updates to the documentation, comprehensive test coverage, and integration into the federation middleware. Review feedback highlights an opportunity to improve type safety in the middleware's exception wrapper by replacing a type assertion with a more specific record type when accessing the meter provider.
The custom collection error wrapper reads the optional meter provider from its handler parameters. Put that optional telemetry hook on the internal ErrorHandlers bound so the wrapper no longer needs a local type assertion. fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request implements OpenTelemetry metrics for ActivityPub collection requests, covering request counts, dispatch durations, and item counts for both built-in and custom collections. The feedback identifies two locations in the custom collection handler where totalItems values—which may be bigint types—require explicit conversion to number to ensure type safety and compatibility with the metric recording functions.
Custom collection counters can originate from number or bigint values. The handler already normalizes them through the totalItems accessor, and these metric writes now make that number conversion explicit at the measurement boundary too. fedify-dev#777 (comment) fedify-dev#777 (comment) Assisted-by: Codex:gpt-5.5
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces OpenTelemetry metrics for ActivityPub collection requests, including counters for requests and histograms for dispatch duration, page items, and total items. It updates the federation handlers and middleware to record these metrics across built-in and custom collections, adds comprehensive unit and integration tests, and updates the documentation to reflect these new observability features. I have no feedback to provide as there were no review comments.
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
This PR adds OpenTelemetry metrics for ActivityPub collection handling as part of #316 and resolves #741.
Fedify already traced collection dispatch, but operators could not graph collection request rate, dispatcher latency, materialized item counts, or reported
totalItemsvalues when traces were sampled. The new metrics expose those signals with bounded attributes only, so custom route names, collection IDs, cursors, actor identifiers, and full URLs stay out of metric labels.Changes
activitypub.collection.request,activitypub.collection.dispatch.duration,activitypub.collection.page.items, andactivitypub.collection.total_items.Verification
mise check && mise test:deno && mise test:node