Opened 3 years ago

Closed 2 years ago

#19842 closed enhancement (worksforme)

offer a `LenientParser` option with metrics-lib

Reported by: iwakeh Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Provide another parser option, quote from #19170#comment:7

make use of the descriptor.parser and descriptor.reader properties and supply a different non-ascci-accepting parser, let's call it LenientParser, as well as a LenientReader.

  • Necessary change in CollecTor would be to set the descriptor.parser and descriptor.reader properties to the LenientParser class.
  • Necessary change in metrics-lib would be the addition of the LenientParser, which consist mostly in providing additional ParserHelper methods that accept non-ascii and calling these in the appropriate places; most of the code will be the same as in the current, stricter implementation. Also a LenientReader would have to be supplied.

That way we could switch between implementations.
Users of metrics-lib would also have another option.

Child Tickets

Change History (2)

comment:1 Changed 2 years ago by karsten

Status: newneeds_information

Hmm, I don't know how to proceed with this issue. A few thoughts:

  • We should only use new properties for new implementations of a parser or reader, not for a config option. If we want a config option, let's add a method to the descriptor source.
  • DescriptorReaderImpl internally uses a DescriptorParser for parsing descriptor. If we just want to switch to another parser, let's create a method DescriptorReader#setDescriptorParser(). Or let's use DescriptorSourceFactory internally to create a parser instance rather than instantiating DescriptorParserImpl directly.
  • The case of non-ASCII characters in statistics lines seems rather special, and doesn't seem worth adding an option that 99.9% of users won't care about. We could generalize and provide an option to not fail descriptor parsing if a single line cannot be parsed, or something like that.
  • If non-ASCII characters in statistics line are spec-conformant, let's fix metrics-lib by default. I don't recall whether that's the case. But if it's a spec violation, let's treat this case as any other spec violation.
  • How much do we still care about this issue? Is there a current use case that would justify putting in the effort? If yes, okay. But if not, let's just close this ticket.

comment:2 Changed 2 years ago by iwakeh

Resolution: worksforme
Status: needs_informationclosed

It seems there is no need currently for the different type of parser.

Closing.

Note: See TracTickets for help on using tickets.