Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Parser into specialized parsers for each StatsD syntax #576

Open
pedro-stanaka opened this issue Sep 12, 2024 · 2 comments
Open
Labels
enhancement library Issues pertaining to the use of the packages as a library more than the exporter itself

Comments

@pedro-stanaka
Copy link
Contributor

pedro-stanaka commented Sep 12, 2024

Description

The current Parser implementation in pkg/line/line.go handles multiple types of syntax (DogStatsD, InfluxDB, Librato, SignalFX) within a single parser. This makes the code complex and harder to maintain. We propose refactoring the Parser to separate parsers for each syntax type.

Goals

  1. Improve Code Maintainability: By separating the parsing logic for different syntax types, the code will be easier to understand and maintain.
  2. Allow for improvements for specific cases: The current parser is quite slow and heavy on allocations for parsing tags for Dogstatsd, for example. We could after refactoring ensure that we optimize that specific parser.
  3. Avoid Breaking Changes: Ensure that the refactoring does not break existing projects that use this repository as a library.

Proposed Solution

  1. Create Separate Parsers: Implement individual parsers for DogStatsD, InfluxDB, Librato, and SignalFX.
  2. Parser Interface: Define a Parser interface that each specific parser will implement.
  3. Stop-gap MultiParser: Define a MultiParser that will mimic the current behavior of the parser, and will check if the incoming line uses which type of tagging. This is necessary, if your instance needs to understand more than one type of syntax.
  4. Factory Method: Implement a factory method to create the appropriate parser based on configuration, here we could make sure to use only the specific parser, if only one syntax is enabled.
@matthiasr
Copy link
Contributor

matthiasr commented Sep 12, 2024 via email

@matthiasr matthiasr added enhancement library Issues pertaining to the use of the packages as a library more than the exporter itself labels Sep 15, 2024
@matthiasr
Copy link
Contributor

Oh joy, I misremembered how we are currently treating tags when a specific flavor is disabled. If you send a line that uses a disabled tagging format, e.g. DogStatsD tags when DogStatsD parsing is disabled, we accept the line and ignore the tags. To get this behavior, we have to still parse for all formats.

I think this is strange, and we should break it. It makes the parsing very inflexible and inefficient, for a use case that nobody ought to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement library Issues pertaining to the use of the packages as a library more than the exporter itself
Projects
None yet
Development

No branches or pull requests

2 participants