expfmt: throw error when metric's name not provided instead panicing

[Alhanaqtah]

This PR enhances extfmt behavior when incorrect metrics privided by user — metrics without name. Now TextParser throws error "invalid metrics name" instead of panicing (#3374)

@roidelapluie

[Alhanaqtah]

@roidelapluie could you review PR, please?)

[ywwg]

This change looks good to me. Can you add a test to exercise this case so we can catch regressions? And can you sign your commit to pass the DCO check? Thanks!

[ywwg]

Hi, I would like to do a release of prom/common and this would be a nice fix to include, do you think you can add the test and fix DCO soon?

[Alhanaqtah]

Hi, I would like to do a release of prom/common and this would be a nice fix to include, do you think you can add the test and fix DCO soon?

I've done everything. You can merge it)

P.S.
I also extended (*TextParser).reset() with p.currentMetric assignment to nil so that testcases wouldn't affect each other

[Alhanaqtah]

@ywwg Could you merge PR, please?)

[Alhanaqtah]

Hello! I would be glad if someone of you approved PR)
@roidelapluie @gotjosh @ArthurSens

[jan--f]

Apologies for the delay here.
I don't think this is quite correct. This test case already exists at https://github.com/prometheus/common/pull/811/files#diff-f94ed3d7ef59cf8de9ae97ae9e654cadda29ab3257af8a5112d93e2290e6bad2R804, though without HELP and TYPE annotation. In these lines though you do pass a metric name.
On current main this new test case no longer causes a panic, so I assume this was fixed elsewhere and the parser can take the name from the HELP/TYPE lines. Without those lines the parser returns the expected parsing error.

[jan--f]

It is entirely possible though that this new test case covers a corner case in the parser we need to fix. I reached out for more eyes on this.

[jan--f]

So yes this is a corner case previously not tested for, thanks for fixing this.
Can you please rebase on current main?

[jan--f]

Suggested change

	# HELP backupmonitor_size The size of the given backup.
	# TYPE backupmonitor_size counter
	{host="local", dir="alpha"} 1834194837
	{host="remote", dir="beta"} 133638016
	# HELP metric a metric.
	# TYPE metric counter
	{label="bla"} 3.14

Just to be consistent with previous test cases.

[Alhanaqtah]

I came across the fact that the error is not displayed at the values as in the previous metrics, so I decided to use these values:

# HELP backupmonitor_size The size of the given backup.
# TYPE backupmonitor_size counter
{host="local", dir="alpha"} 1834194837
{host="remote", dir="beta"} 133638016

[jan--f]

Not sure I follow. The request is make the test case consistent with what we use in the other test cases. The naming itself should have no effect on the case being tested.

[Alhanaqtah]

So yes this is a corner case previously not tested for, thanks for fixing this.

Can you please rebase on current main?

Done :)

[jan--f]

This looks like an accident?

[jan--f]

Not sure I follow. The request is make the test case consistent with what we use in the other test cases. The naming itself should have no effect on the case being tested.