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

[pkg/ottl] Add map literals #32388

Closed
evan-bradley opened this issue Apr 15, 2024 · 9 comments
Closed

[pkg/ottl] Add map literals #32388

evan-bradley opened this issue Apr 15, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request pkg/ottl priority:p2 Medium

Comments

@evan-bradley
Copy link
Contributor

Component(s)

pkg/ottl

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

Currently there is no way in OTTL to create a new map. This makes certain use cases difficult and limits the language's expressiveness.

Describe the solution you'd like

Use {"key": value, [...]} as a map literal syntax, similar to how it looks in JavaScript, JSON, or Go structs. In particular, I would like the following to be possible when retaining the original log body while parsing a log:

- set(body, {"_raw": body})
- set(body["mykey"], "myvalue")
- merge_maps(body, ParseJSON(body["_raw"]))

Describe alternatives you've considered

Instead of map literals, we could add a Map function with optional parameters to create keys in the initial map. However, I think maps are important enough in OTLP to justify adding map literals to the language.

Additional context

Implementation will broadly look something like the following:

  1. Update buildLexer in grammar.go to understand map literals.
  2. Update the AST in grammar.go to capture keys and values in the map.
  3. Update the parser to create pdata map objects from map literal nodes in the AST.
Copy link
Contributor

Pinging code owners for pkg/ottl: @TylerHelmuth @kentquirk @bogdandrutu @evan-bradley. See Adding Labels via Comments if you do not have permissions to add labels yourself.

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.

@bacherfl
Copy link
Contributor

@evan-bradley if this issue is still available, I would like to work on this

@evan-bradley
Copy link
Contributor Author

That would be great @bacherfl, thank you.

@evan-bradley
Copy link
Contributor Author

A few thoughts in response to #34168 (review):

  1. Just like lists, map literals don't need to be indexible for the foreseeable future. We can add that functionality in a follow-up if there is user interest.
  2. I would see the full-ish allowable syntax for a map literal looking like the following:
    {
        "str": "string",
        "int": 123,
        "bool": true,
        "double": 1.23,
        "math": value_int * 3
        "converter": ConvertCase(name, "upper"),
        "enum": METRIC_DATA_TYPE_SUM
    }
    

@TylerHelmuth
Copy link
Member

How will non-static values be handled?

@TylerHelmuth
Copy link
Member

Do list literals allow Enums Converters Editors, and MathExpressions?

@evan-bradley
Copy link
Contributor Author

How will non-static values be handled?

They'll be evaluated as part of the statement. I think that's the most intuitive way to handle it.

Do list literals allow Enums Converters Editors, and MathExpressions?

They do. It's worth noting that editors are explicitly disallowed; the parser will catch them and return an error if it sees one.

@TylerHelmuth
Copy link
Member

Ok, we should match the same types as list.

TylerHelmuth pushed a commit that referenced this issue Aug 12, 2024
**Description:** This PR extends the OTTL grammar to support map
literals

**Link to tracking Issue:** #32388

**Testing:** Unit tests; E2E Tests

**Documentation:** Extended the docs with a section about the added data
type

---------

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this issue Sep 12, 2024
**Description:** This PR extends the OTTL grammar to support map
literals

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

**Testing:** Unit tests; E2E Tests

**Documentation:** Extended the docs with a section about the added data
type

---------

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request pkg/ottl priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

4 participants