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

ParseJSON doesn't allow leading square bracket #33535

Closed
Marty1703 opened this issue Jun 13, 2024 · 7 comments
Closed

ParseJSON doesn't allow leading square bracket #33535

Marty1703 opened this issue Jun 13, 2024 · 7 comments
Assignees
Labels
bug Something isn't working pkg/ottl priority:p2 Medium

Comments

@Marty1703
Copy link

Marty1703 commented Jun 13, 2024

Component(s)

pkg/ottl

What happened?

Description

My (valid) JSON starting with a squared bracket isn't processed correctly by the ParseJSON function

Steps to Reproduce

Add transform

       transform:
        log_statements:
        - context: log
          statements:
          - set(attributes["MyBody"], ParseJSON(attributes["Body"])) where IsMatch(attributes["Body"], "^\\[")

Process JSON
[
{"id":"123","description": "test description"},
{"id":"456","description": "test description"}
]

Expected Result

Correction parsing of JSON array objects.

Actual Result

2024-06-13T06:13:00.722Z error logs/processor.go:54 failed processing logs {"kind": "processor", "name": "transform", "pipeline": "logs", "error": "failed to execute statement: set(attributes[\"MyBody\"], ParseJSON(attributes[\"Body\"])) where IsMatch(attributes[\"Body\"], \"^\\\\[\"), ReadMapCB: expect { or n, but found [, error found in #1 byte of ...|[{\"id\":\"123|..., bigger context ...|[{\"id\":\"123\",\"description\":\"test description\"},|..."} github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/logs.(*Processor).ProcessLogs github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor@v0.101.0/internal/logs/processor.go:54 go.opentelemetry.io/collector/processor/processorhelper.NewLogsProcessor.func1 go.opentelemetry.io/collector/processor@v0.101.0/processorhelper/logs.go:48 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/processor/processorhelper.NewLogsProcessor.func1 go.opentelemetry.io/collector/processor@v0.101.0/processorhelper/logs.go:56 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/processor/processorhelper.NewLogsProcessor.func1 go.opentelemetry.io/collector/processor@v0.101.0/processorhelper/logs.go:56 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/processor/processorhelper.NewLogsProcessor.func1 go.opentelemetry.io/collector/processor@v0.101.0/processorhelper/logs.go:56 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/processor/processorhelper.NewLogsProcessor.func1 go.opentelemetry.io/collector/processor@v0.101.0/processorhelper/logs.go:56 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/consumer.ConsumeLogsFunc.ConsumeLogs go.opentelemetry.io/collector/consumer@v0.101.0/logs.go:25 go.opentelemetry.io/collector/internal/fanoutconsumer.(*logsConsumer).ConsumeLogs go.opentelemetry.io/collector@v0.101.0/internal/fanoutconsumer/logs.go:62 go.opentelemetry.io/collector/receiver/otlpreceiver/internal/logs.(*Receiver).Export go.opentelemetry.io/collector/receiver/otlpreceiver@v0.101.0/internal/logs/otlp.go:41 go.opentelemetry.io/collector/pdata/plog/plogotlp.rawLogsServer.Export go.opentelemetry.io/collector/pdata@v1.8.0/plog/plogotlp/grpc.go:88 go.opentelemetry.io/collector/pdata/internal/data/protogen/collector/logs/v1._LogsService_Export_Handler.func1 go.opentelemetry.io/collector/pdata@v1.8.0/internal/data/protogen/collector/logs/v1/logs_service.pb.go:311 go.opentelemetry.io/collector/config/configgrpc.(*ServerConfig).toServerOption.enhanceWithClientInformation.func9 go.opentelemetry.io/collector/config/configgrpc@v0.101.0/configgrpc.go:439 go.opentelemetry.io/collector/pdata/internal/data/protogen/collector/logs/v1._LogsService_Export_Handler go.opentelemetry.io/collector/pdata@v1.8.0/internal/data/protogen/collector/logs/v1/logs_service.pb.go:313 google.golang.org/grpc.(*Server).processUnaryRPC google.golang.org/grpc@v1.64.0/server.go:1379 google.golang.org/grpc.(*Server).handleStream google.golang.org/grpc@v1.64.0/server.go:1790 google.golang.org/grpc.(*Server).serveStreams.func2.1

Collector version

v.0.101.0

Environment information

Environment

Container image: otel/opentelemetry-collector-contrib:0.101.0

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

@Marty1703 Marty1703 added bug Something isn't working needs triage New item requiring triage labels Jun 13, 2024
Copy link
Contributor

Pinging code owners:

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

@evan-bradley
Copy link
Contributor

evan-bradley commented Jun 14, 2024

Thanks for reporting this @Marty1703. I checked and this happens because we expect that ParsJSON parses and returns a map. I think it's fair to expect it should return whatever type matches the top-level object in the JSON string, or at a minimum that we document this limitation. The docs do cover that we return a pcommon.Map, but not that other top-level types aren't somehow coerced into a map.

@evan-bradley evan-bradley added priority:p2 Medium and removed needs triage New item requiring triage labels Jun 14, 2024
@TylerHelmuth
Copy link
Member

For reference, stanza has a separate JSON Array parser for these situations

@bacherfl
Copy link
Contributor

bacherfl commented Jul 3, 2024

Hi! I would like to pick up this issue. Would you rather have this functionality added to the current ParseJSON function, or as a separate ParseJSONArray function, like in stanza? CC @evan-bradley @TylerHelmuth

@evan-bradley
Copy link
Contributor

evan-bradley commented Jul 3, 2024

@djaglowski Can you offer any insights as to why Stanza has both json_parser and json_array_parser? Have users found that more intuitive than a single JSON parsing operator? My gut instinct is that we should parse everything in ParseJSON and do type checking on the output with other OTTL functions, but I could be missing something.

@djaglowski
Copy link
Member

I think it comes down to the idea that json-structured logs are nearly always objects, and because often the intention of parsing a json-structured log is to write the result into attributes, which is a map.

I think there could be value in a generic json parsing function in addition to the two typed functions, though it's never been needed in stanza.

@bacherfl
Copy link
Contributor

bacherfl commented Jul 4, 2024

I have now made a PR to enable the existing ParseJSON function to also parse top level array values. Feel free to have a look at the PR and comment your thoughts on if we should do it this way or if a separate ParseJSONArray is the better option after all

TylerHelmuth pushed a commit that referenced this issue Jul 25, 2024
…3908)

**Description:** This PR enables the `ParseJSON` function to also handle
top level arrays.

**Link to tracking Issue:** #33535 

**Testing:** Added unit and e2e tests

**Documentation:** Adapted the documentation of the `ParseJSON` function
to indicate that this can either return a `pcommon.Map` or
`pcommon.Slice` value

---------

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
bug Something isn't working pkg/ottl priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

5 participants