Skip to content

[Nginx] Improve generated error message#8909

Merged
ruflin merged 1 commit intoelastic:mainfrom
ruflin:improve-nginx-error-message
Feb 14, 2024
Merged

[Nginx] Improve generated error message#8909
ruflin merged 1 commit intoelastic:mainfrom
ruflin:improve-nginx-error-message

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Jan 17, 2024

In #8762 error log generation was added. The message was a random message generated. This changes it to have a partially more realistic message generated based on some actual logs. The enum values for the different fields are hardcoded. It would be nice to have for host, ip and paths some partially random generation that could be used as function with cardinality.

In elastic#8762 error log generation was added. The message was a random message generated. This changes it to have a partially more realistic message generated based on some actual logs. The enum values for the different fields are hardcoded. It would be nice to have for host, ip and paths some partially random generation that could be used as function with cardinality.
@ruflin ruflin requested a review from a team as a code owner January 17, 2024 08:27
max: 100000
- name: request.method
enum: ["POST", "GET", "PUT", "DELETE"]
- name: request.path
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aspacca Are there any generate functions for host names, ip addresses and paths that could be used?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruflin

I realized it is not documented in the docs of corpus generator, but you can define an example on the fields.yml for a keyword field and the random generated values will follow the example "pattern".

we currently support only the following patterns:
a.dot.separated.string
a-dash-separated-string
an_underline_separated_string
a space separated field

the generated value will match the "separator" in the pattern, and how many "words" there are

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share a full yaml example on how this should work in the scenario here for the request path? Tried with dots but in the outcome didn't have dots.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fields.yml

- name: request.path
  type: keyword
  example: a.path

no need to define anything specific in config.yml for request.path, indeed, you cannot have an enum on request.path if you want the example from fields.yml to be used. maybe that was the reason why it didn't work for you

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now the following in fields.yml:

- name: request.path
  type: keyword
  example: my.dotted.path

I expected to get paths with 2 dots inside, what I get is values like forgerkingmercurymask.

@aspacca Can you open a PR against this PR to provide a working example, I think that will be quicker.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aspacca Can you open a PR against this PR to provide a working example, I think that will be quicker.

@ruflin I identified the bug and it's related specifically to using . as separator (https://github.com/elastic/elastic-integration-corpus-generator-tool/blob/main/pkg/genlib/generator_interface.go#L404)

If you replace . with _, - or a space in fields.yml is going to work.

I'll create a PR to fix the bug.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@ruflin ruflin requested a review from ali786XI January 17, 2024 08:27
@ruflin ruflin self-assigned this Jan 17, 2024
@elasticmachine
Copy link
Copy Markdown

elasticmachine commented Jan 17, 2024

🚀 Benchmarks report

Package nginx 👍(2) 💚(0) 💔(0)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
access 0 1730.1 1730.1 ( - %) 👍
error 0 11764.71 11764.71 ( - %) 👍

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Jan 17, 2024

/test benchmark fullreport

@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

cc @ruflin

Copy link
Copy Markdown
Contributor

@ali786XI ali786XI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ruflin ruflin merged commit 2567489 into elastic:main Feb 14, 2024
@ruflin ruflin deleted the improve-nginx-error-message branch February 14, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants