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

Span status from HTTP code: Do not set status message if it can be inferred #1681

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Removed the exported `SimpleSpanProcessor` and `BatchSpanProcessor` structs.
These are now returned as a SpanProcessor interface from their respective constructors. (#1638)

### Fixed
- Do not set span status message in `SpanStatusFromHTTPStatusCode` if it can be inferred from `http.status_code`. (#1681)

## [0.18.0] - 2020-03-03

### Added
Expand Down
49 changes: 27 additions & 22 deletions semconv/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,29 +264,34 @@ var validRangesPerCategory = map[int][]codeRange{
// SpanStatusFromHTTPStatusCode generates a status code and a message
// as specified by the OpenTelemetry specification for a span.
func SpanStatusFromHTTPStatusCode(code int) (codes.Code, string) {
spanCode, valid := func() (codes.Code, bool) {
category := code / 100
ranges, ok := validRangesPerCategory[category]
if !ok {
return codes.Error, false
}
ok = false
for _, crange := range ranges {
ok = crange.contains(code)
if ok {
break
}
}
if !ok {
return codes.Error, false
}
if category > 0 && category < 4 {
return codes.Unset, true
}
return codes.Error, true
}()
spanCode, valid := validateHTTPStatusCode(code)
if !valid {
return spanCode, fmt.Sprintf("Invalid HTTP status code %d", code)
}
return spanCode, fmt.Sprintf("HTTP status code: %d", code)
return spanCode, ""
}

// Validates the HTTP status code and returns corresponding span status code.
// If the `code` is not a valid HTTP status code, returns span status Error
// and false.
func validateHTTPStatusCode(code int) (codes.Code, bool) {
category := code / 100
ranges, ok := validRangesPerCategory[category]
if !ok {
return codes.Error, false
}
ok = false
for _, crange := range ranges {
ok = crange.contains(code)
if ok {
break
}
}
if !ok {
return codes.Error, false
}
if category > 0 && category < 4 {
return codes.Unset, true
}
return codes.Error, true
}
9 changes: 8 additions & 1 deletion semconv/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,8 +696,15 @@ func TestHTTPAttributesFromHTTPStatusCode(t *testing.T) {
func TestSpanStatusFromHTTPStatusCode(t *testing.T) {
for code := 0; code < 1000; code++ {
expected := getExpectedCodeForHTTPCode(code)
got, _ := SpanStatusFromHTTPStatusCode(code)
got, msg := SpanStatusFromHTTPStatusCode(code)
assert.Equalf(t, expected, got, "%s vs %s", expected, got)

_, valid := validateHTTPStatusCode(code)
if !valid {
assert.NotEmpty(t, msg, "message should be set if error cannot be inferred from code")
} else {
assert.Empty(t, msg, "message should not be set if error can be inferred from code")
}
}
}

Expand Down