Skip to content

Conversation

pavelsavara
Copy link
Member

  • split aspnetcore.components.render_diff into
    • aspnetcore.components.render_diff.duration
    • aspnetcore.components.render_diff.size
  • rename aspnetcore.components.navigation to aspnetcore.components.navigate to match the trace name
  • rename aspnetcore.components.event_handler to aspnetcore.components.handle_event.duration to match the trace name
  • rename aspnetcore.components.update_parameters to aspnetcore.components.update_parameters.duration
  • update unit tests

Fixes #62555

…render_diff.duration and aspnetcore.components.render_diff.size

- rename aspnetcore.components.navigation to aspnetcore.components.navigate to match the trace name
- rename aspnetcore.components.event_handler to aspnetcore.components.handle_event.duration to match the trace name
- rename aspnetcore.components.update_parameters to aspnetcore.components.update_parameters.duration
- update unit tests
@pavelsavara pavelsavara added this to the 10.0-preview7 milestone Jul 16, 2025
@pavelsavara pavelsavara requested a review from JamesNK July 16, 2025 14:11
@pavelsavara pavelsavara self-assigned this Jul 16, 2025
@pavelsavara pavelsavara added the area-blazor Includes: Blazor, Razor Components label Jul 16, 2025
@pavelsavara pavelsavara requested a review from noahfalk July 16, 2025 14:12
@pavelsavara pavelsavara marked this pull request as ready for review July 16, 2025 18:03
@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 18:03
@pavelsavara pavelsavara requested a review from a team as a code owner July 16, 2025 18:03
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aligns Blazor diagnostic metric names with OTEL conventions, splits the render diff metric into separate duration and size histograms, and updates corresponding unit tests.

  • Renamed several metrics to include .duration or match trace names
  • Added a new histogram for diff size and its bucket boundaries
  • Updated unit tests to use renamed metrics and include the new batch size metric

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Shared/Metrics/MetricsConstants.cs Added bucket boundaries for diff length histogram
src/Components/Components/src/ComponentsMetrics.cs Renamed metrics, added _batchSize histogram, updated recording logic
src/Components/Components/test/ComponentsMetricsTest.cs Updated tests for renamed metrics and new batch size metric
Comments suppressed due to low confidence (1)

src/Components/Components/test/ComponentsMetricsTest.cs:384

  • [nitpick] Consider adding a test to verify that the batch size histogram is also recorded on error paths and that recorded values fall into expected buckets.
        Assert.True(batchSizeMeasurements[0].Value > 0);

public static readonly IReadOnlyList<double> LongSecondsBucketBoundaries = [0.01, 0.02, 0.05, 0.1, 0.2, 0.5, 1, 2, 5, 10, 30, 60, 120, 300];

// For blazor rendering, which should be very fast.
public static readonly IReadOnlyList<double> BlazorRenderingSecondsBucketBoundaries = [0.000001, 0.00001, 0.0001, 0.001, 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10];
Copy link
Member

Choose a reason for hiding this comment

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

This has more buckets than other durations (18 compared to 14). I don't know if that's a problem or not, but it's an inconsistency.

private readonly Histogram<double> _eventDuration;
private readonly Histogram<double> _parametersDuration;
private readonly Histogram<double> _batchDuration;
private readonly Histogram<int> _batchSize;
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing that there would never be more than int.MaxValue items rendered in a batch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[API Proposal] Diagnostic metrics for Blazor

3 participants