Logstash: Style guide practices when adding metrics to logstash plugins

[purbon]

Hi,
during the work I've been doing lately on #5607 I've noticed a few patterns that might DRY'ed somehow in order to provide:

• Usage of similar vocabulary in terms of, singulars vs plurals.
• Same words for common terms, for example to name errors, etc.
• Provide uniform structure, so consumers will expect similar organization.

I'm sure as far as we get into adding more metrics to filters and outputs we will encounter patterns that might makes sense to uniform.

Patterns

1. Dealing with errors happening during execution path.

For this I started creating a namespaced metric, @metric_errors = metric.namespace(:errors) that later on is used to track error situations like exceptions, for example. This makes all error situation be namespaced.

example:

[jordansissel]

Style guide proposal:

• choose names which are user-focused, not developer-focused. For example, I believe users will be more familiar with the word "error" than they might with "exception" - so for counting code exceptions, we should count them as "errors"
• metric names must be lowercase
• metric names should be only one word
• if metric names are not a single word, they should be snake_cased.
• if the unit is not a discrete count of things, then the metric should be name_unit where name is the name of the metric and unit is the unit of measurement. For example, "duration_millis" or "read_bytes".
• Use the name plural as shorthand for a count unit. Using "event" as an example, where we would describe 'count of event' we should use the plural events
• use namespaces instead of multiple words. If a multi-word metric name has each word being a "kind of" a category, split it into namespaces. For example, instead of connection_errors -- we can ask "What kind of errors? Connection errors", then we can create an errors namespace with connections in it.
• namespace names should be plural and be the name of what they contain. For example, an errors namespace contains metrics for errors.
• namespaces hierarchy should put aspects/categories first, where possible. For an example: { "errors": { "connections": 5 } is better than { "connections": { "errors": 5 } }

[ph]

@jordansissel excellent writes up, I will finish the review of the PR and update this issue also.

I think we should name this as best practices, even if some of theses check could be done in the code. I think we shouldn't do it, since we want metric to be fast.

[jordansissel]

agreed, my though my preference is to call it a style guide, not best practices

[jordansissel]

What are others' thoughts on this style guide I proposed a few comments above?

[suyograo]

+1. Looks good

[purbon]

@jordansissel thanks for your feedback, will update the current PR's following your advices. I'm not sure about the naming you propose, but I guess I'm good with it, basic idea for me was to extrapolate similar patterns for all to follow, so I guess naming is ok 😸 .

@ph what do you mean by

even if some of theses check could be done in the code

? curious to know more what you are thinking here.

[ph]

I've reread the plugins PR and I am +1 with your proposal @jordansissel.

even if some of theses check could be done in the code

We could add to the metric library normalization of keys and adding validation.

edited for typos.

[acchen97]

Thanks for this @jordansissel.

This style guide looks like good material to include within a general "Adding and Collecting New Plugin Metrics" section in the docs.

[jordansissel]

@acchen97 ++. Target this for 5.0.0?

[acchen97]

++ added to 5.0 docs meta issue (https://github.com/elastic/logstash/issues/5148) to track.