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

1173 zipkin exporter v1 json support #1

Open
wants to merge 83 commits into
base: master
Choose a base branch
from

Conversation

robwknox
Copy link
Owner

@robwknox robwknox commented Nov 16, 2020

Description

Adds support for the Zipkin Exporter v1 API json format . Since this brings the number of supported formats to 3 (v1-json, v2-json, v2-protobuf) and with v1-thrift on the way, the PR includes notable refactoring of the exporter to introduce the concept of an Encoder.

Breaking changes

  • ZipkinSpanExporter() signature changes:
    • url => endpoint to be consistent with related env var OTEL_EXPORTER_ZIPKIN_ENDPOINT and package DEFAULT_ENDPOINT
    • ipv4 => local_node_ipv4 to more clearly denote this is for the local node endpoint
    • ipv6 => local_node_ipv6 to more clearly denote this is for the local node endpoint
    • local_node_port: added. This value was being improperly parsed from the url when it is supposed to represent the local node listening port. Added param so that it can be passed in along with the local node ip addresses if desired.
    • retry removed for now to avoid confusion since the logic is not currently implemented.

Not a breaking change but ZipkinSpanExporter(service_name) is now optional to follow spec. Default is 'unknown'.

Fixes open-telemetry#1173

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

  • Manual testing against local zipkin server:
    • v1-json support
    • v1-json / v2-json / v2-protobuf output comparison testing
    • regression testing of v2-json and v2-protobuf
  • Unit tests have been greatly reworked & expanded.
  • Full tox run

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

…RANSPORT_FORMAT_JSON/PROTOBUF vals to not overload w/ Content-Type
….5 + a bug fix related to max_tag_value_length in the encoders
…ce_id() and encode_span_id() and base Encoder class
@robwknox robwknox changed the base branch from 1175_zipkin_exporter_v2_protobuf_support to master November 23, 2020 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants