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

Add resource_type tag #4140

Merged
merged 7 commits into from
Jan 3, 2025
Merged

Add resource_type tag #4140

merged 7 commits into from
Jan 3, 2025

Conversation

ankur22
Copy link
Contributor

@ankur22 ankur22 commented Dec 19, 2024

What?

A new resource_type tag is being added to allow users to view and filter metrics based on the chrome resource types, which are currently defined as:

  • "Document"
  • "Stylesheet"
  • "Image"
  • "Media"
  • "Font"
  • "Script"
  • "TextTrack"
  • "XHR"
  • "Fetch"
  • "Prefetch"
  • "EventSource"
  • "WebSocket"
  • "Manifest"
  • "SignedExchange"
  • "Ping"
  • "CSPViolationReport"
  • "Preflight"
  • "Other"

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

Linked: grafana/xk6-browser#1551

@ankur22 ankur22 requested a review from a team as a code owner December 19, 2024 16:37
@ankur22 ankur22 requested review from inancgumus and joanlopez and removed request for a team December 19, 2024 16:37
@@ -180,6 +180,7 @@ func (m *NetworkManager) emitRequestMetrics(req *Request) {
if state.Options.SystemTags.Has(k6metrics.TagURL) {
tags = handleURLTag(m.mi, req.URL(), req.method, tags)
}
tags = tags.With("resource_type", req.ResourceType())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two things that I'm not sure of:

  1. This isn't a system tag, but should it be?
  2. This list can change at a moments notice by the chrome team, should we map them from the chrome ResourceType to a k6 version of it. We would be able to detect if we stop matching on any of the types and log a warning, plus create a unit test to catch this as early as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

We would be able to detect if we stop matching on any of the types and log a warning, plus create a unit test to catch this as early as possible.

This sounds like the way to go! 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a way to validate the ResourceTypes.

@ankur22 ankur22 requested a review from mstoykov December 19, 2024 16:41
@ankur22 ankur22 force-pushed the add/resource-type-tag branch 2 times, most recently from dafc212 to 0f7b08a Compare December 20, 2024 12:39
A new resource_type tag is being added to allow users to view and
filter metrics based on the chrome resource_types.
It's a duplicate of CDP's network.ResourceType.
@ankur22 ankur22 force-pushed the add/resource-type-tag branch from 0f7b08a to a573f2c Compare January 3, 2025 10:18
@ankur22 ankur22 merged commit b888b0a into master Jan 3, 2025
29 checks passed
@ankur22 ankur22 deleted the add/resource-type-tag branch January 3, 2025 11:34
@ankur22 ankur22 added this to the v0.56.0 milestone Jan 6, 2025
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.

4 participants