TextParser: HELP line optional?

[eaglerainbow]

Hi Brian,

This is in reference to promregator/promregator#73, where a tricky situation with HELP lines happens. Here is the situation in a nutshell:

• Promregator parses Prometheus' Exposition Format requests and aggregates/forwards them.
• For this, Promregator has its own Prometheus' Exposition Format parser. The parser is tolerant enough to accept metrics without a # HELP line.
• The format itself explicitly permits that both # HELP and # TYPE are optional (see also https://prometheus.io/docs/instrumenting/exposition_formats/#grouping-and-sorting, first sentence there).
• Promregator's parser outputs its data in the MetricFamilySamples format.
• After a lot of processing, in the end the class TextFormat (of this project here) is used to generate a proper Exposition Format string again, which then is sent to Prometheus.
• NPE due to missing # HELP statement in the scraped metrics promregator/promregator#73 documents a situation, where a metric without # HELP line is processed through the system - and fails.
• As

  client_java/simpleclient_common/src/main/java/io/prometheus/client/exporter/common/TextFormat.java

  Line 23 in 1950cff

  writer.write("# HELP ");

  is not conditional and the metricFamilySamples.help is null, an NPE is raised.

I also stumbled over the check at

client_java/simpleclient/src/main/java/io/prometheus/client/SimpleCollector.java

Line 165 in 1950cff

if (b.help.isEmpty()) throw new IllegalStateException("Help hasn't been set.");

which prevents that any "java-natively" generated metrics comes without a "help" string. I used blame and that line dates back to the very first commits in the repository. The commit's description does not give me any indication why a help string must be enforced. I also could not find any indication in the javadocs that this is enforced at all (or why this is the case).

In general, I am quite in favor that metrics should have a help. However, the TextFormat class currently only implements a subset of the specification and for my case this causes compatibility issues.

What do you think about the situation?

• Is it possible to relax (only) the TextFormat class in that sense? (a PR would not be rocket science - only the unit tests would be a little tricky).
• Was there a special intention behind the idea to enforce help strings even though the protocol is more generous?

Thanks for your insights!

[brian-brazil]

Per the client library guidelines, an empty help string should be accepted (though I don't see a need to support null). If that's not the case, please send a PR.

…

On Sat 15 Sep 2018, 21:27 Nico Schmoigl, ***@***.***> wrote: Hi Brian, This is in reference to promregator/promregator#73 <promregator/promregator#73>, where a tricky situation with HELP lines happens. Here is the situation in a nutshell: - Promregator parses Prometheus' Exposition Format requests and aggregates/forwards them. - For this, Promregator has its own Prometheus' Exposition Format parser. The parser is tolerant enough to accept metrics without a # HELP line. - The format is defined in such a way that both # HELP and # TYPE are optional (see also https://prometheus.io/docs/instrumenting/exposition_formats/#grouping-and-sorting, first sentence there). - Promregator's parser outputs its data in the MetricFamilySamples format. - After a lot of processing, in the end the class TextFormat (of this project here) is used to generate a proper Exposition Format string again, which then is sent to Prometheus. - promregator/promregator#73 <promregator/promregator#73> documents a situation, where a metric *without* # HELP line is processed through the system - and fails. - As https://github.com/prometheus/client_java/blob/1950cff385518eae45651df47547f80547d72c1a/simpleclient_common/src/main/java/io/prometheus/client/exporter/common/TextFormat.java#L23 is not conditional and the metricFamilySamples.help is null an NPE is raised. I also stumbled over the check at https://github.com/prometheus/client_java/blob/1950cff385518eae45651df47547f80547d72c1a/simpleclient/src/main/java/io/prometheus/client/SimpleCollector.java#L165 which prevents that any "java-natively" generated metrics comes without a "help" string. I used blame and that line dates back to the very first commits in the repository. The commit's description does not give me any indication why a help string must be enforced. In general, I am quite in favor that metrics *should* have a help. However, the TextFormat class currently only implements a subset of the specification and in my situation, this causes compatibility issues. What do you think about the situation? - Is it possible to relax (only) the TextFormat class in that sense? - Was there a special intention behind the idea to enforce help strings even though the protocol is more generous? Thanks for your insights! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#425>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGyTdsJsFZoCLPST7PGXnqbIhGeJfrKCks5ubWLNgaJpZM4Wqhzk> .

[eaglerainbow]

Thanks for the very quick response!
I can confirm that the empty string (as help) is accepted (see also below).
Unfortunately, I mean the null case of metricFamilySamples.help, as setting it to "" (empty string) still means that the # HELP line is being rendered.

Example:

  @Test
  public void testNoHelp() throws IOException {
    LinkedList<Collector.MetricFamilySamples> ll = new LinkedList<MetricFamilySamples>();
    ll.add(new MetricFamilySamples("nohelp", Type.GAUGE, "", new LinkedList<Sample>()));
    TextFormat.write004(writer, Collections.enumeration(ll));
    assertEquals("# TYPE nohelp gauge\n", writer.toString());
  }

In this coding, the assertEquals fails, as an additional # HELP nohelp is being generated.

I am not so deep in the protocol's consumption (from its specification, you can't decide it): Is it ensured that the non-existent # HELP line is always treated exactly the same way as a # HELP line containing the empty string? If that was the case, I would feel comforted again. Then, in case I get a non-existing # HELP line, I would convert it to an empty string on a # HELP line for you...

[brian-brazil]

Missing help should be identical to enjoy empty help.

…

On Sun 16 Sep 2018, 09:19 Nico Schmoigl, ***@***.***> wrote: Thanks for the very quick response! I can confirm that the empty string (as help) is accepted (see also below). Unfortunately, I mean the null case of metricFamilySamples.help, as setting it to "" (empty string) still means that the # HELP line is being rendered. Example: @test public void testNoHelp() throws IOException { LinkedList<Collector.MetricFamilySamples> ll = new LinkedList<MetricFamilySamples>(); ll.add(new MetricFamilySamples("nohelp", Type.GAUGE, "", new LinkedList<Sample>())); TextFormat.write004(writer, Collections.enumeration(ll)); assertEquals("# TYPE nohelp gauge\n", writer.toString()); } In this coding, the assertEquals fails, as an additional # HELP nohelp is being generated. I am not so deep in the protocol's consumption (from its specification, you can't decide it): Is it ensured that the non-existent # HELP line is always treated exactly the same way as a # HELP line containing the empty string? If that was the case, I would feel comforted again. Then, in case I get a non-existing # HELP line, I would convert it to an empty string on a # HELP line for you... — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#425 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGyTdrDKLbulJlOtgVxIOoTWc3o9If1Fks5ubgmDgaJpZM4Wqhzk> .

[eaglerainbow]

Ok, thanks. Then I will convert the nulls into empty strings.
See also commits attached to promregator/promregator#73