Skip to content

Remove extra emit of request metrics - #5630

Merged
ankur22 merged 2 commits into
masterfrom
fix/extra-emit-of-request-metrics
Apr 2, 2026
Merged

Remove extra emit of request metrics#5630
ankur22 merged 2 commits into
masterfrom
fix/extra-emit-of-request-metrics

Conversation

@ankur22

@ankur22 ankur22 commented Feb 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What?

Removes call to emit metrics during redirects. The emitting of the request metric is done at the end of the onRequest function in any case.

Why?

This is incorrect behaviour. The redirectChain grows after ever redirect by one. So in the old behaviour it would loop over all the elements in redirectChain and emit a metric for that element, even if it has already done son in a previous redirect -- i.e. it is compounding.

Checklist

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

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

@ankur22
ankur22 temporarily deployed to azure-trusted-signing February 9, 2026 22:01 — with GitHub Actions Inactive
@ankur22
ankur22 temporarily deployed to azure-trusted-signing February 9, 2026 22:04 — with GitHub Actions Inactive
@ankur22
ankur22 force-pushed the fix/extra-emit-of-request-metrics branch from fca34be to 7d765fe Compare February 9, 2026 23:17
@ankur22
ankur22 temporarily deployed to azure-trusted-signing February 9, 2026 23:24 — with GitHub Actions Inactive
@ankur22
ankur22 temporarily deployed to azure-trusted-signing February 9, 2026 23:26 — with GitHub Actions Inactive
@ankur22 ankur22 added this to the v1.7.0 milestone Feb 10, 2026
@ankur22
ankur22 marked this pull request as ready for review February 17, 2026 13:17
@ankur22
ankur22 requested a review from a team as a code owner February 17, 2026 13:17
@ankur22
ankur22 requested review from codebien and szkiba and removed request for a team February 17, 2026 13:17
szkiba
szkiba previously approved these changes Feb 17, 2026
View reviewed changes

@szkiba szkiba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from fix/redirect-request-events to master February 24, 2026 10:33
@ankur22
ankur22 dismissed szkiba’s stale review February 24, 2026 10:33

The base branch was changed.

@ankur22
Remove extra redirect request metric emits
2cded6e 
This is incorrect behaviour. The redirectChain grows after ever
redirect by one. So in the old behaviour it would loop over all the
elements in redirectChain and emit a metric for that element, even if
it has already done son in a previous redirect -- i.e. it is
compounding.
@ankur22
ankur22 force-pushed the fix/extra-emit-of-request-metrics branch from 7d765fe to 2cded6e Compare March 4, 2026 12:51
@ankur22
ankur22 requested a review from szkiba March 4, 2026 12:51
@ankur22
ankur22 temporarily deployed to azure-trusted-signing March 4, 2026 12:58 — with GitHub Actions Inactive
@ankur22
ankur22 temporarily deployed to azure-trusted-signing March 4, 2026 12:59 — with GitHub Actions Inactive
@joanlopez

joanlopez commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Moving it to the next milestone to unlock the ongoing release process.

@joanlopez joanlopez modified the milestones: v1.7.0, v2.0.0 Mar 24, 2026
@ankur22
ankur22 temporarily deployed to azure-trusted-signing April 2, 2026 15:45 — with GitHub Actions Inactive
@ankur22
ankur22 temporarily deployed to azure-trusted-signing April 2, 2026 15:48 — with GitHub Actions Inactive
@ankur22
ankur22 merged commit ff093a9 into master Apr 2, 2026
53 checks passed
@ankur22
ankur22 deleted the fix/extra-emit-of-request-metrics branch April 2, 2026 16:15
@BrewTestBot BrewTestBot mentioned this pull request May 11, 2026
Merged
@mstoykov mstoykov mentioned this pull request May 15, 2026
Merged
4 of 8 tasks
inancgumus pushed a commit that referenced this pull request May 15, 2026
@mstoykov @ankur22
Remove extra redirect request metric emits (#5630) (#5987)
6e122c9 
This is incorrect behaviour. The redirectChain grows after ever
redirect by one. So in the old behaviour it would loop over all the
elements in redirectChain and emit a metric for that element, even if
it has already done son in a previous redirect -- i.e. it is
compounding.

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

This branch was previously deployed

1 inactive deployment
azure-trusted-signing — 395f0c5f Deployed Apr 2, 2026 by ankur22 via sign-packages #8753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codebien codebien codebien approved these changes
@szkiba szkiba szkiba approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@ankur22 @joanlopez @codebien @szkiba