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

[exporter/loki] Auto replace labels with dot to underscore #14113

Closed
rogeriospinaintelipost opened this issue Sep 13, 2022 · 19 comments
Closed
Labels
enhancement New feature or request exporter/loki Loki Exporter priority:p2 Medium

Comments

@rogeriospinaintelipost
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Loki doesn`t accept labels that contains dots, so I have to duplicate my attributes via processors copying the value from attribute with dot to other attribute with underscore.

Ex: host.name copy to -> host_name

      resource:
        attributes:     
          - action: insert
            key: host_name
            from_attribute: host.name
    
          - action: insert
            key: namespace
            from_attribute: k8s.namespace.name
   
          - action: insert
            key: container_name
            from_attribute: k8s.container.name

Describe the solution you'd like

Would be nice if the str replace happens automatically by the lokiexporter

Describe alternatives you've considered

No response

Additional context

@jpkrohling

@rogeriospinaintelipost rogeriospinaintelipost added enhancement New feature or request needs triage New item requiring triage labels Sep 13, 2022
@jpkrohling jpkrohling self-assigned this Sep 13, 2022
@evan-bradley evan-bradley added priority:p2 Medium exporter/loki Loki Exporter and removed needs triage New item requiring triage labels Sep 14, 2022
@github-actions
Copy link
Contributor

Pinging code owners: @gramidt @jpkrohling. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@disfluxly
Copy link

This is absolutely needed, especially with the change between the Legacy Exporter & the New Exporter and the way hints are now set by the Attributes Processor, whereas previously you could do dot to underscore label mapping in the Legacy Exporter.

@jpkrohling
Copy link
Member

jpkrohling commented Nov 29, 2022

@mar4uk, can you keep this on your radar?

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Jan 30, 2023
@gouthamve gouthamve removed the Stale label Jan 31, 2023
@fkalinowski
Copy link

fkalinowski commented Mar 1, 2023

Hi, something similar to resource_to_telemetry_conversion feature available in exporter/prometheusremotewriteexporter would also be of great help !

BTW: there is already a translator to convert from OpenTelemetry naming convention into Prometheus (here Loki) naming convention cf. https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/translator/prometheus

@jpkrohling
Copy link
Member

@kovrus, @mar4uk, I think it would be a good idea to consider this, especially if this exists on the Prometheus remote writer exporter. What do you think?

@kovrus
Copy link
Member

kovrus commented Mar 1, 2023

+1 We can add it. I guess, we would keep it behind some config parameter and not make it default behavior?

@kago-dk
Copy link

kago-dk commented Mar 1, 2023

If Loki doesn't accept labels that contain dots, in which scenario would you need to send labels with dots? I cannot rule out that I am reading the description correctly.

@jpkrohling
Copy link
Member

@kovrus, didn't we have this translation already in the past? Did it get lost?

@disfluxly
Copy link

@kago-dk - Loki doesn't accept label index mappings with . in them. So currently, if you want to index service.name, you need to use the attributes processor to effectively duplicate service.name into service_name, as shown in the original description. You then have to add the newly created attribute (service_name) to the loki.attribute/resource.labels attribute/resourceattribute. This is seen better here: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/lokiexporter/README.md

Adding in the auto-conversion option would effectively remove the necessity of having to relabel all your necessary attributes/labels.

So basically I'm 🙏 over here for this to be added :)

@kovrus
Copy link
Member

kovrus commented Mar 2, 2023

@kovrus, didn't we have this translation already in the past? Did it get lost?

In the legacy exporter, we used the attributes mapping as part of the exporter config to create loki labels.
In the new exporter with labels hints we use the attributes processor to do the same thing.

+1 We can add it. I guess, we would keep it behind some config parameter and not make it default behavior?

I am actually hesitating now about supporting this feature. It would require a major config change, also I guess, it would make sense to control what loki labels should be created, so exclude/include filters probably would have to be added as well, etc. Also, changing attribute names (replacing . with _) can be achieved with the transform processor.

@kovrus
Copy link
Member

kovrus commented Mar 2, 2023

Adding in the auto-conversion option would effectively remove the necessity of having to relabel all your necessary attributes/labels.

FYI We also use the hints approach (using the attributes processor) to provide tenant information to the exporter.

@jpkrohling
Copy link
Member

I agree, @kovrus. Besides, Loki may end up supporting dots in the future, causing yet another breaking change for this exporter. I'd stick with explicit vs. implicit here.

@gouthamve
Copy link
Member

gouthamve commented Mar 6, 2023

I want to see if we can revisit this. I don't see . support as a priority item on the Loki roadmap and I think it might be months before Loki supports it.

And even if it does support it, we still have to support the existing users of Loki that haven't or cannot upgrade.

Also, changing attribute names (replacing . with _) can be achieved with the transform processor.

Could you give an example? The best I can get to is the following:

Without Transform processor

processors:
  attributes:
    actions:
    - action: insert
      key: event_domain
      from_attribute: event.domain
    - action: insert
      key: loki.attribute.labels
      value: event_domain

  resource:
    attributes:
    - action: insert
      key: service_name
      from_attribute: service.name
    - action: insert
      key: service_namespace
      from_attribute: service.namespace
    - action: insert
      key: loki.resource.labels
      value: service_name, service_namespace

With Transform Processor:

processors:
  transform:
    log_statements:
      - context: resource
        statements:
          - set(attributes["service_name"], attributes["service.name"])
          - set(attributes["service_namespace"], attributes["service.namespace"])
      - context: log
        statements:
          - set(attributes["event_domain"], attributes["event.domain"])

  attributes:
    actions:
    - action: insert
      key: loki.attribute.labels
      value: event_domain

  resource:
    attributes:
    - action: insert
      key: loki.resource.labels
      value: service_name, service_namespace

(I couldn't find how to do "insert" with OTTL). Again, this is from me reading the docs but I couldn't find a simpler way to do it.

While this is smaller, its only by a bit and its just as cumbersome as before. If I want to do 6 labels in Loki, I have to manually escape them and specify the config.

@gouthamve
Copy link
Member

Hi, I synced with @kovrus offline and we decided to support this usecase. We will escape . to _ before sending to Loki.

@fkalinowski
Copy link

Hi @gouthamve, great news that you will support automatic escape of . to _ !
What about resource_to_telemetry_conversion feature available in exporter/prometheusremotewriteexporter, will you support it also ?

@gouthamve
Copy link
Member

That is a more risky thing to support as it could lead to massive cardinality. However, given a resource is more or less stable, I am inclined to consider it.

Could you open a new issue for this so we can have a discussion there?

@kovrus
Copy link
Member

kovrus commented Mar 7, 2023

@mar4uk would like to take it over, so it is assigned to her now.

codeboten pushed a commit that referenced this issue Mar 22, 2023
Loki doesn't support label names containing dots.
Added label names normalization to follow Prometheus label names standard. Dots in label names will be converted to underscores before sending them to Loki

Link to tracking Issue: #14113
---------

Co-authored-by: Evan Bradley <[email protected]>
@jpkrohling
Copy link
Member

This seems to have been done. I'm closing, if anything is still pending let me know and I'll reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporter/loki Loki Exporter priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

8 participants