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

Extending Attributes processor #1170

Closed
ccaraman opened this issue Jun 23, 2020 · 14 comments
Closed

Extending Attributes processor #1170

ccaraman opened this issue Jun 23, 2020 · 14 comments
Assignees
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p3 Lowest

Comments

@ccaraman
Copy link
Contributor

Currently the design for the attributes processor is limited. It supports some basic functions(insert/upsert/update/delete/hash/...) however the configuration is cumbersome and limits any richer functionalities being added. ex. Should there be mathematical operations? What about the new attribute types (nested arrays and maps)? Should action represent the value source and add a concept of operator(insert/update/...)?

Please refer to #979 for an example

This issue tracks figuring out the plan for the attributes processor.

Several open questions:

  • To avoid breaking changes, should there be versioning of this processor?

Related issues to be tagged

  • Breaking config change process
@ccaraman ccaraman added the help wanted Good issue for contributors to OpenTelemetry Service to pick up label Jun 23, 2020
tigrannajaryan pushed a commit that referenced this issue Jun 24, 2020
Note: All of the cudos go to @dneray for the logic/testing/effort in #981

This pr is based off of it with the following modifications

- The config looks like
```
- key : <key to use for applying the rule too>
  pattern: <the regex pattern with named submatchers>
  action: extract
```
I think that the original PR is going in the right way for what we can the internal logic to be but as the issue #979 and #1170 indicate we need to take a step back and redesign the attributes processor (or make it clear of its limitations).  This pr unblocks the required functionality. 

**Link to tracking Issue:** #979 

**Link to follow up issue:** #1170 

**Testing:** Added tests for extracting.

**Documentation:** Updated processor readme.
@tigrannajaryan
Copy link
Member

@ccaraman is this now resolved or there is more work to do?

wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this issue Jul 13, 2020
Note: All of the cudos go to @dneray for the logic/testing/effort in open-telemetry#981

This pr is based off of it with the following modifications

- The config looks like
```
- key : <key to use for applying the rule too>
  pattern: <the regex pattern with named submatchers>
  action: extract
```
I think that the original PR is going in the right way for what we can the internal logic to be but as the issue open-telemetry#979 and open-telemetry#1170 indicate we need to take a step back and redesign the attributes processor (or make it clear of its limitations).  This pr unblocks the required functionality. 

**Link to tracking Issue:** open-telemetry#979 

**Link to follow up issue:** open-telemetry#1170 

**Testing:** Added tests for extracting.

**Documentation:** Updated processor readme.
@ccaraman
Copy link
Contributor Author

@tigrannajaryan - there is work to be done with this issue - I'll be adding some of the examples of what the configuration could look like that would have a richer feature set.

@ccaraman
Copy link
Contributor Author

Ex of a configuration entry that would better work for maps See #979 (comment) for full context

# Delete nested
del attr["foo"]["bar"]

# Add dictionary
attr["bar"] = {"one": "two"}

# Overwrite list
attr["list"] = [1,2,3]

# Append to list
attr["list"] += [4,5,6]

# regex
attr["http.url"]  = /^(?P<http_protocol>.*):\/\/(?P<http_domain>.*)\/(?P<http_path>.*)(\?|\&)(?P<http_query_params>.*)/

# from another key
attr["string key"] = attr["another key"]

@ccaraman
Copy link
Contributor Author

Or separating action and operations into two different concepts from comment https://github.com/open-telemetry/opentelemetry-collector/pull/981/files#r426882973

SetValue -> {AttributeValue, Action}
SetValueFromAttribute -> {FromAttribute, Action}
SetValuesFromRegexp -> ....

@jrcamp
Copy link
Contributor

jrcamp commented Jul 21, 2020

If we didn't want to invent our own syntax another option would be to use an existing embeddable scripting language. Fluentbit, for instance, can use lua:

https://docs.fluentbit.io/manual/pipeline/filters/lua

Another option is Tengo: https://github.com/d5/tengo

@jrcamp jrcamp added this to the GA 1.0 milestone Jul 21, 2020
@flands flands removed this from the GA 1.0 milestone Oct 1, 2020
@jrcamp jrcamp added the priority:p3 Lowest label Mar 10, 2021
@jrcamp jrcamp added this to the Phase2-GA-Roadmap milestone Mar 10, 2021
@jrcamp
Copy link
Contributor

jrcamp commented Mar 10, 2021

See Bogdan's expression prototype #2408

@bhautikpip
Copy link
Contributor

@jrcamp @ccaraman I can work with you to contribute on this issue. However, I am not super familiar with collector code and looking to contribute for my first issue. Any pointers or help would be appreciated.

@alolita
Copy link
Member

alolita commented May 18, 2021

@bhautikpip can you start working on a design proposal for this issue?

@alolita
Copy link
Member

alolita commented May 18, 2021

This issue is related to the design proposal in #3185

@carlosalberto
Copy link
Contributor

I can take this item in case @bhautikpip is not available anymore ;)

@carlosalberto
Copy link
Contributor

Ping @alolita @bhautikpip

@bhautikpip
Copy link
Contributor

bhautikpip commented May 23, 2021

@carlosalberto @alolita I can start looking on this once I am back as I am currently not available due to traveling. @carlosalberto feel free to take this.

@carlosalberto
Copy link
Contributor

Thanks!

@carlosalberto carlosalberto self-assigned this May 24, 2021
@bogdandrutu
Copy link
Member

This should be part of #3185. We need first to understand what components do we offer, etc.

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this issue Nov 11, 2021
…open-telemetry#1170)

* Don't consider unset env var an error during detection

* update CHANGELOG
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this issue Apr 27, 2023
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this issue Oct 9, 2024
…y-operator (open-telemetry#1170)

* Allowed ephemeral storage to operator

* Resolve chart version conflicts

* Bump version

* Added key and comments

* Revert changes in crd

* fix spaces

* Disable ephemeral-storage by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up priority:p3 Lowest
Projects
None yet
Development

No branches or pull requests

8 participants