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

Change Zipkin receiver behavior according to host ingestion status #148

Merged

Conversation

pjanotti
Copy link
Contributor

This change makes the Zipkin receiver respond to received data according to the host ingestion status.

This is part of #76

@codecov-io
Copy link

codecov-io commented Jul 10, 2019

Codecov Report

Merging #148 into master will increase coverage by 0.63%.
The diff coverage is 74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   73.05%   73.69%   +0.63%     
==========================================
  Files          95       95              
  Lines        5789     5827      +38     
==========================================
+ Hits         4229     4294      +65     
+ Misses       1350     1318      -32     
- Partials      210      215       +5
Impacted Files Coverage Δ
...servability/observabilitytest/observabilitytest.go 86.95% <0%> (-13.05%) ⬇️
config/configmodels/configmodels.go 0% <0%> (ø) ⬆️
receiver/zipkinreceiver/factory.go 100% <100%> (ø) ⬆️
observability/observability.go 50% <100%> (+8.33%) ⬆️
receiver/zipkinreceiver/trace_receiver.go 72.66% <91.42%> (+16.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c7c3ca...47e0826. Read the comment docs.

config/configmodels/configmodels.go Outdated Show resolved Hide resolved
config/configmodels/configmodels.go Outdated Show resolved Hide resolved
observability/observability.go Outdated Show resolved Hide resolved
var responseStatusCode int
var zPageMessage string
if zr.backPressureState == configmodels.EnableBackPressure {
responseStatusCode = http.StatusServiceUnavailable
Copy link
Member

Choose a reason for hiding this comment

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

In the future we can also try to be smart and calculate queue drain speed and the time it will be required for it to drain to certain reasonable level and report that in "Retry-After" header. Not for this PR, just something to consider for later.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM

Paulo Janotti added 3 commits July 11, 2019 14:47
This change makes the Zipkin receiver respond to received data according to the host status regarding ingestion.
@pjanotti pjanotti force-pushed the zipkin-receiver-ingestion-response branch from 9048b08 to 47e0826 Compare July 11, 2019 21:50
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@pjanotti pjanotti merged commit 8721e3b into open-telemetry:master Jul 15, 2019
@pjanotti pjanotti deleted the zipkin-receiver-ingestion-response branch July 15, 2019 22:10
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.

4 participants