Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Disable JSON support in OTLP receiver #1178

Closed
wants to merge 1 commit into from
Closed

Disable JSON support in OTLP receiver #1178

wants to merge 1 commit into from

Conversation

nilebox
Copy link
Member

@nilebox nilebox commented Jun 24, 2020

Description:
Disable support for JSON payload in OTLP receiver.
Return a JSON response with error message 'application/json' encoding is not supported for requests with Content-Type: application/json.

Link to tracking Issue: Fixes #1177

Testing: Unit test

Documentation: -

// Explicitly handle "application/json" encoding to return custom human friendly error
// See issue https://github.com/open-telemetry/opentelemetry-proto/issues/129
// describing why we don't support JSON at the moment.
gatewayruntime.WithMarshalerOption("application/json", newJSONUnsupportedMarshaler()),
Copy link
Member Author

@nilebox nilebox Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we just override default * marshaler, it will return a binary protobuf response with the status message "Unexpected EOF", which might be confusing when users sent JSON payload with Content-Type: application/json header, so I've implemented a custom marshaler which returns a JSON response with a better error message:

{
  "error": "'application/json' encoding is not supported",
  "code": 3,
  "message": "'application/json' encoding is not supported"
}

@nilebox
Copy link
Member Author

nilebox commented Jun 24, 2020

/cc @bogdandrutu @james-bebbington

@@ -0,0 +1,54 @@
// Copyright 2020 The OpenTelemetry Authors
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the year was removed from the license, but make addlicense still adds it.

// describing why we don't support JSON at the moment.
gatewayruntime.WithMarshalerOption("application/json", newJSONUnsupportedMarshaler()),
// Override default JSON handling with Protobuf
gatewayruntime.WithMarshalerOption(gatewayruntime.MIMEWildcard, protobufMarshaler),
Copy link
Member Author

@nilebox nilebox Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change in this PR - by default, gRPC Gateway is using JSON marshaler for any content type other than explicitly configured above, see https://github.com/grpc-ecosystem/grpc-gateway/blob/ee3ef70b7777cde4e61e4e224cb11e92beecee6a/runtime/marshaler_registry.go#L78. Here I changed it to use Protobuf marshaler by default.

Alternatively, we may require always setting Content-Type: application/x-protobuf explicitly and return an error for * wildcard here.

@nilebox
Copy link
Member Author

nilebox commented Jun 30, 2020

Closing this PR for now, since there's still an open discussion in #1177

@nilebox nilebox closed this Jun 30, 2020
@nilebox nilebox changed the title Disable JSON support in OTLP receiver Official JSON support in OTLP receiver Jun 30, 2020
@nilebox nilebox changed the title Official JSON support in OTLP receiver Disable JSON support in OTLP receiver Jun 30, 2020
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…y#1178)

Bumps [boto3](https://github.com/boto/boto3) from 1.20.45 to 1.20.46.
- [Release notes](https://github.com/boto/boto3/releases)
- [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst)
- [Commits](boto/boto3@1.20.45...1.20.46)

---
updated-dependencies:
- dependency-name: boto3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON/HTTP support in OTLP receiver
1 participant