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

[Agent] Support labels dedot in k8s provider #27019

Closed
ChrsMark opened this issue Jul 22, 2021 · 7 comments
Closed

[Agent] Support labels dedot in k8s provider #27019

ChrsMark opened this issue Jul 22, 2021 · 7 comments
Assignees
Labels
Agent autodiscovery kubernetes Enable builds in the CI for kubernetes Team:Integrations Label for the Integrations team

Comments

@ChrsMark
Copy link
Member

ChrsMark commented Jul 22, 2021

We need to support label's dedoting or use flattened type as proposed at #26801 (comment). We need to verify which option works best for us so the scope of this issue is to investigate the option of flattened and evaluate if it could be used instead of dedoting the labels (and annotations) as it was the case in Beats.

cc: @MichaelKatsoulis @akshay-saraswat @exekias

@ChrsMark ChrsMark added Team:Integrations Label for the Integrations team autodiscovery Agent kubernetes Enable builds in the CI for kubernetes labels Jul 22, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@ChrsMark
Copy link
Member Author

ChrsMark commented Sep 1, 2021

Regarding the pros/cons of flattened type it seems that the only issue here is that flattened type is not yet supported in Kibana (elastic/kibana#25820).
We are not hitting any licensing issue with Agent and also k8s labels are stored as keywords so we are not affected by numeric value support issue.

Speaking of this, @exekias @jsoriano do you think it is a good time now to consider using flattened. I guess it depends on how quick the support in Kibana will come?

It is true that currently we don't use any labels in our dashboards/visualisations but maybe users want to create custom visualisation based on labels, so I think this is something quite important for us in order to adopt this type.

@ChrsMark
Copy link
Member Author

ChrsMark commented Sep 1, 2021

Note that this is tightly coupled with the implementation of #24495. If we aim to use the same interfaces/libs (see the draft PR linked to #24495) with Beats in order to generate metadata then the dettoting comes along:

labelMap = generateMapSubset(accessor.GetLabels(), r.config.IncludeLabels, r.config.LabelsDedot)

If we conclude to use flattened though in Agent then we can just hardcode the dedot config to false when we call the libs from Agent. Btw sth I miss here is where these fields are defined in Agent, do you know that @exekias? Do we have sth similar to

for Agent 🤔 ? It seems that elastic/package-spec#199 is related.

@exekias
Copy link
Contributor

exekias commented Sep 6, 2021

If we conclude to use flattened though in Agent then we can just hardcode the dedot config to false when we call the libs from Agent. Btw sth I miss here is where these fields are defined in Agent, do you know that @exekias? Do we have sth similar to

for Agent 🤔 ? It seems that elastic/package-spec#199 is related.

I don't think Agent is defining these anywhere, it relies on default metrics-* and logs-* mappings, which will default to keyword for these labels. This won't work as soon as you want to use a custom type like flattened. cc @mtojek @jsoriano for awareness

@jsoriano
Copy link
Member

jsoriano commented Sep 6, 2021

Btw sth I miss here is where these fields are defined in Agent

At the moment some common agent fields, as the ones for cloud or containers, are defined on each integration in the fields/agent.yml field of each integration, but this doesn't scale long term and is error-prone.

It seems that elastic/package-spec#199 is related.

Yes, this would be the long-term solution.

@ChrsMark
Copy link
Member Author

ChrsMark commented Sep 7, 2021

It seems that even if the flattened type looks promising it cannot be used in this specific case at the moment because of:

  1. It doesn't support Kibana visualisation etc, sth that we consider quite important for k8s labels.
  2. We dont have a way to define it in the mappings until [Discuss] Elastic Packages: Introduce schema for data providers package-spec#199 is landed.

In this I would suggest to move it forward using the old dedot approach we had in Beats and try to move to flattened type when it is better supported (sounds highly possible that we will be dealing with a breaking change with this but we should be able to come with a nice deprecation plan). @exekias @jsoriano let me know if that's ok for you.

@mukeshelastic @akshay-saraswat it would be nice if we could have an estimation about 1, and maybe check again with Kibana folks what is the status and if they plan to work on this soon.
@mtojek this is another case that would take advantage of elastic/package-spec#199 :).

@ChrsMark
Copy link
Member Author

Closed by #27691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent autodiscovery kubernetes Enable builds in the CI for kubernetes Team:Integrations Label for the Integrations team
Projects
None yet
Development

No branches or pull requests

4 participants