-
Notifications
You must be signed in to change notification settings - Fork 248
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
API: Add HTTP to Status convenience mapper #157
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* based on google apis docs: https://cloud.google.com/apis/design/errors#handling_errors There's also https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md which mentions: "Servers must not use this table to determine an HTTP status code to use; the mappings are neither symmetric nor 1-to-1.", so...
* note that .http_to_status is a convenience method, not part of API spec
duonoid
requested review from
bai,
dazuma,
elskwid,
fbogsany,
luvtechno and
mwear
as code owners
November 21, 2019 00:23
fbogsany
reviewed
Nov 21, 2019
fbogsany
reviewed
Nov 21, 2019
fbogsany
approved these changes
Nov 22, 2019
mwear
added a commit
that referenced
this pull request
Jan 9, 2020
* Change SDK::Trace::TracerFactory to subclass API's TracerFactory * to allow access to #http_text_format * Add API OpenTelemetry::Adapter * for access to global config, e.g., OpenTelemetry::Adapter.tracer * Add sinatra adapter Gemfile, gemspec * Add sinatra adapter, middleware, and extension * Add sinatra server example * e.g., $ docker-compose run ex-adapter-sinatra bundle install $ docker-compose run ex-adapter-sinatra $ ./start_server.sh $ ruby sinatra.rb Expected: console output of SpanData * Don't specify :kind, it will default to :internal * Don't specify :with_parent, current_span will be default parent * Refine unused variables * Pass attributes in span initializer instead of via set_attribute * Set span.status in trace_response * Add Trace::Status.from_http_status * based on google apis docs: https://cloud.google.com/apis/design/errors#handling_errors There's also https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md which mentions: "Servers must not use this table to determine an HTTP status code to use; the mappings are neither symmetric nor 1-to-1.", so... * Fix occasional test failure in sdk batch_span_processor_test * RE: "failures on heavily loaded CI server" -- increase delta experimentally, by 10x * Sync recommendations from faraday branch * should move #tracer_version up/shared if it is approved * Fix occasional test failure in sdk batch_span_processor_test (+100ms) * RE: failure in jruby -- increase delta to 100ms * restore 'normal' case (-1ms), since it wasn't demonstrating failure * Revert "Add Trace::Status.from_http_status" This reverts commit 1f3d458. Superceded by #157 * Update to use http_to_status() * Fix uninitialized constant Status * Avoid requiring changes to API (OpenTelemetry::Adapter) * Avoid installing multiple times (registry/install once) * problem: sinatra middleware is registered multiple times, creating a span per registration * Synchronize changes with faraday instrumentation * Tests: Change Gemfile, gemspec, add Rakefile * Update gemspec * Ignore Appraisal's *.gemfile.lock * Add appraisal(s) for running tests against multiple versions example: $ bundle exec appraisal install $ bundle exec appraisal rake test * Add Circle CI config * Test: Adapters::Sinatra * Test: Adapters::Sinatra::Adapter * Add test_helper * Replace registry with class.installed? problem: if .install is called (globally) elsewhere, then ':installed' is not returned * Revert "Replace registry with class.installed?" This reverts commit 165f455. bad commit, tests occasionally failing. * Replace registry with class.installed? * Refine Sinatra::Adapter to remove #tracer_factory * Avoid passing :name and :version in example * :name and :version are internal concerns * Add a note regarding return values * alternatives: return nil and/or unspecified value from `register_tracer_extension` * intention: useful for testing purposes to determine which code path was followed * Refine tracer.in_span call to pass attributes inline * more efficient, due to mutex in `span.set_attribute` * Fix failing circleci builds (appraisal + bundler) * failing: bundler-2.1.2 RE: /usr/local/lib/ruby/2.6.0/rubygems.rb:283:in `find_spec_for_exe': Could not find 'bundler' (2.1.2) required by `$BUNDLER_VERSION` * see bundler issue #7489 * last known good: bundler-2.0.2 * Fix failing circleci - jruby * circleci build is failing after jruby:9.2.9-jre * theory: jruby includes (vendored) its own bundler version * circleci: Remove appraisal support, use latest bundler * TODO: re-enable appraisal support once appraisal issue #162 is solved * circleci: Workaround failing appraisal+bundler(2.1.x)+ruby-2.7 issue * don't run bundle exec appraisal rake test under ruby-2.7 * NOTE: build was failing under jruby:latest (9.3.x) as well * Fix: circleci config oops Co-authored-by: Don Morrison <elskwid@users.noreply.github.com> Co-authored-by: Matthew Wear <matthew.wear@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Adds a
Trace::Status.http_to_status(code)
method to facilitate mapping from http status code toStatus
.Background
Related