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

Add support for _meta in fields.yml #42

Closed
ruflin opened this issue Sep 8, 2020 · 6 comments · Fixed by #73
Closed

Add support for _meta in fields.yml #42

ruflin opened this issue Sep 8, 2020 · 6 comments · Fixed by #73
Assignees

Comments

@ruflin
Copy link
Contributor

ruflin commented Sep 8, 2020

In elastic/elasticsearch#61941 a standardised way on how to use _meta information on fields is documented. On the spec side these standard _meta entries should be supported and validated.

@ruflin
Copy link
Contributor Author

ruflin commented Sep 8, 2020

@ph @skh As soon as we add support for _meta to the fields.yml, we need to follow up on the Kibana side to also support it properly. I assume today we don't.

@exekias
Copy link
Contributor

exekias commented Oct 30, 2020

Let's move this forward, elastic/elasticsearch#61941 was merged, we want to add metric_type and unit to the _meta field in templates. I think we have two options here:

  • Making _meta explicit in fields.yml, providing freedom to add these two fields, and any other one that users may need.
  • Making _meta an implementation detail to fields.yml, having unit and metric_typeas top level items.

I'm leaning towards second option, things would look like this in fields.yml:

- name: total.norm.pct
  type: scaled_float
  metric_type: gauge
  unit: percent
  format: percent
  description: |
     The percentage of CPU time spent by the process since the last event. This value is normalized by the number of CPU cores and it ranges from 0 to 100%.

Note: Eventually format will not be needed, but for now I think we should not change this.

That would translate into a mapping like:

"system": {
  "properties": {
    "total": {
      "properties": {
        "norm": {
          "properties": {
            "pct": {
              "scaling_factor": 1000,
              "type": "scaled_float"
              "_meta": {
                "metric_type": "gauge",
                "unit": "percent",
              }
            }
          }
        }
      }
    }

Valid values for metric_type are: gauge and counter.
Valid values for unit are: percent, byte, d, h, m, s, ms, micros, nanos

@ruflin
Copy link
Contributor Author

ruflin commented Nov 2, 2020

I'm also in favor of option 2 as it feels less abstract. The disadvantage is that we decouple the fields.yml more from the actual index template structure but this is already partially the case.

@ycombinator
Copy link
Contributor

ycombinator commented Nov 2, 2020

I'm also in favor of option 2 for the following reasons:

  • it keeps field definitions in fields.ymls a bit abstracted away from ES details,
  • in a way all the other properties of the field are metadata anyway, so having a special _meta just for these two properties seems odd, and
  • it keeps the structure flat in the YAML which, I think, is a tiny bit nicer developer experience,

@exekias
Copy link
Contributor

exekias commented Nov 2, 2020

Spec merged, next steps:

  • I will follow up with the team to start adding these fields as we migrate integrations
  • @ruflin @ph @skh will you take on supporting this on the Kibana side of fields.yml?

@ph
Copy link

ph commented Nov 2, 2020

Created elastic/kibana#82273 as a followup.

rw-access pushed a commit to rw-access/package-spec that referenced this issue Mar 23, 2021
* Implementing lint subcommand

* Silence usage on validation errors

* Re-fetching validator package

* Sort imports

* Re-formatting

* Move SilenceUsage to root cmd

* Fixing mistake made in rebase

* Refactoring: extracting FindPackageRoot

* Adding godocs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants