Skip to content

Conversation

aknuds1
Copy link
Contributor

@aknuds1 aknuds1 commented Aug 25, 2025

Add expfmt.NewTextParser function that allows providing name validation scheme. I think this was an omission in #808, since it added the in practice mandatory expfmt.TextParser.scheme field ("Defaults to the invalid UnsetValidation"), but provides no way for clients outside of the expfmt package to configure it. Through expfmt.NewTextParser, prometheus/common clients can safely create expfmt.TextParsers with a valid name validation scheme.

I'm also including a test for NewTextParser.

Add expfmt.NewTextParser function that allows providing name validation
scheme.

Signed-off-by: Arve Knudsen <[email protected]>
@aknuds1 aknuds1 requested a review from Copilot August 25, 2025 07:38
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 adds a NewTextParser function to the expfmt package that allows clients to create TextParser instances with a specified name validation scheme. This addresses an omission where the TextParser.scheme field was made mandatory but no public constructor was provided for external clients to configure it properly.

  • Adds NewTextParser(model.ValidationScheme) constructor function
  • Updates existing code to use the new constructor instead of zero-value initialization
  • Includes comprehensive test coverage for the new function

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
expfmt/text_parse.go Adds the new NewTextParser constructor function
expfmt/text_parse_test.go Adds test for the new constructor and updates existing tests to use it
expfmt/fuzz.go Updates fuzzing code to use the new constructor
expfmt/decode.go Updates decoder to use the new constructor

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@aknuds1 aknuds1 added the bug label Aug 25, 2025
@aknuds1 aknuds1 requested a review from vesari August 25, 2025 08:20
Copy link
Contributor

@vesari vesari left a comment

Choose a reason for hiding this comment

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

LGTM

@aknuds1 aknuds1 merged commit 9deefba into main Aug 25, 2025
9 checks passed
@aknuds1 aknuds1 deleted the arve/textparser-scheme branch August 25, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants