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

Allow only certain whitelisted chars to be present in messages in order to avoid too much diversity #30

Closed
flaviostutz opened this issue May 12, 2021 · 6 comments · Fixed by #33
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@flaviostutz
Copy link
Member

In some cases the error-info message comes with variable parts in the message, like "message code" or "account code". As the message is a metrics label, each different string then creates whole new metrics, skyrocketing the number of different metrics (they are not summed), leading to memory leak on the client application, creating network bottlenecks and generating tons of data on Prometheus/Cortex or other infrastructure that will handle the metrics unnecessarily.

Some examples are "Account 546775 couldn't not be created", "There was an error processing your request. erroid=538-433.456", "There are 4 pending approvals for your profile".

Proposal

The proposal is to limit the permitted characters to a fixed whitelist through a regex expression, specially removing all numbers present in the message, thus avoiding the type of issues described as examples.

By applying the following regex cleanup code to the messages, the examples would become:

const regex = /[^A-zÀ-ú\s\.\,]+/ig;
p.replaceAll(regex, "Account 546775 couldn't not be created");
  • "Account couldn't not be created"
  • "There was an error processing your request. erroid=-."
  • "There are pending approvals for your profile"

This way, even if the client sets the error-info attribute with numeric variables we won't have metrics diversification as it will be handled as the same metrics/message (after transformation).

What do you think?

@CarlosPanarello
Copy link

We can use this regex for default, but it can be changed with some environment. But limited by 50 chars.

@flaviostutz
Copy link
Member Author

Agree. Maybe init-param "info-regex"?

@CarlosPanarello
Copy link

what about error-info-regex? info-regex maybe is too generic, info about what?
This name will be relation only with error-info http header parameter.

@flaviostutz
Copy link
Member Author

flaviostutz commented May 17, 2021 via email

Ziul added a commit that referenced this issue May 19, 2021
fixes #29 and fixes #30

- support custom regex to filter
- support custom max value
- support to select the correct group of the regex

Signed-off-by: Luiz Oliveira <ziuloliveira@gmail.com>
@Ziul
Copy link
Member

Ziul commented May 20, 2021

I created the PR #33 that accepts up to three environment variables: filter-regex, filter-index, and filter-max-size. With it, we can solve the problem about the max size of the message, apply a regex to the message and even get which matching group of the regex the user desires. To get more aligned with your discussion I can rename the variables to error-info-regex, error-info-index and error-info-max-size.

@CarlosPanarello
Copy link

I created the PR #33 that accepts up to three environment variables: filter-regex, filter-index, and filter-max-size. With it, we can solve the problem about the max size of the message, apply a regex to the message and even get which matching group of the regex the user desires. To get more aligned with your discussion I can rename the variables to error-info-regex, error-info-index and error-info-max-size.

it would be great if you did that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants