Skip to content

Detect HTTP payload format from Content-Type#916

Merged
yurishkuro merged 2 commits intomasterfrom
content-type
Jul 8, 2018
Merged

Detect HTTP payload format from Content-Type#916
yurishkuro merged 2 commits intomasterfrom
content-type

Conversation

@yurishkuro
Copy link
Copy Markdown
Member

Since Zipkin HTTP endpoint has been moved to another port, the only format that Collector HTTP handler recognizes is Thrift, which makes the ?format= URL parameter redundant since the payload type can be inferred from the Content-Type header.

This change removes the format URL parameter and only looks at the Content-Type.

Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 8, 2018

Codecov Report

Merging #916 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #916   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         126    126           
  Lines        6073   6071    -2     
=====================================
- Hits         6073   6071    -2
Impacted Files Coverage Δ
cmd/collector/app/http_handler.go 100% <100%> (ø) ⬆️

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 f83617f...42a06a1. Read the comment docs.

@yurishkuro yurishkuro merged commit 89f3cca into master Jul 8, 2018
@ghost ghost removed the review label Jul 8, 2018
UnableToReadBodyErrFormat = "Unable to process request body: %v"
)

var (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For my own improvement: why the parenthesis?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No strong reason, it looks consistent with const block as declarative section.

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.

3 participants