Skip to content

Commit

Permalink
Update StatusCodeFromHTTP to accept int64 (#6699)
Browse files Browse the repository at this point in the history
The functionality of StatusCodeFromHTTP can be performed on an int64 as
easily as an int. By updating the function to accept an int64 instead of
an int as an argument the callers do not need to "down cast" the value
and potentially handle int64 to int32 truncation issues. This updates
the function to accept an int64 and resolve this issue.

This introduces an int64 to int32 truncation issue in the Jaeger
translator as the returned value of getStatusCodeValFromAttr is used as
a pdata.StatusCode which has an underlying int32 representation. This is
resolved in #6682 where that conversion is made explicit and this bug
will be removed.
  • Loading branch information
MrAlias authored and PaurushGarg committed Dec 14, 2021
1 parent 51fa974 commit d5ea9e4
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const (

// StatusCodeFromHTTP takes an HTTP status code and return the appropriate OpenTelemetry status code
// See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#status
func StatusCodeFromHTTP(httpStatusCode int) pdata.StatusCode {
func StatusCodeFromHTTP(httpStatusCode int64) pdata.StatusCode {
if httpStatusCode >= 100 && httpStatusCode < 399 {
return pdata.StatusCodeUnset
}
Expand Down
8 changes: 2 additions & 6 deletions pkg/translator/jaeger/jaegerproto_to_traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package jaeger // import "github.com/open-telemetry/opentelemetry-collector-cont
import (
"encoding/base64"
"fmt"
"math"
"reflect"
"strconv"

Expand Down Expand Up @@ -252,7 +251,7 @@ func setInternalSpanStatus(attrs pdata.AttributeMap, dest pdata.SpanStatus) {
}
}

func getStatusCodeValFromAttr(attrVal pdata.AttributeValue) (int, error) {
func getStatusCodeValFromAttr(attrVal pdata.AttributeValue) (int64, error) {
var codeVal int64
switch attrVal.Type() {
case pdata.AttributeValueTypeInt:
Expand All @@ -266,10 +265,7 @@ func getStatusCodeValFromAttr(attrVal pdata.AttributeValue) (int, error) {
default:
return 0, fmt.Errorf("invalid status code attribute type: %s", attrVal.Type().String())
}
if codeVal > math.MaxInt32 || codeVal < math.MinInt32 {
return 0, fmt.Errorf("invalid status code value: %d", codeVal)
}
return int(codeVal), nil
return codeVal, nil
}

func getStatusCodeFromHTTPStatusAttr(attrVal pdata.AttributeValue) (pdata.StatusCode, error) {
Expand Down
9 changes: 1 addition & 8 deletions pkg/translator/jaeger/jaegerproto_to_traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestGetStatusCodeValFromAttr(t *testing.T) {
tests := []struct {
name string
attr pdata.AttributeValue
code int
code int64
err error
}{
{
Expand Down Expand Up @@ -78,13 +78,6 @@ func TestGetStatusCodeValFromAttr(t *testing.T) {
code: 0,
err: invalidNumErr,
},

{
name: "invalid-int",
attr: pdata.NewAttributeValueInt(1844674407370955),
code: 0,
err: fmt.Errorf("invalid status code value: 1844674407370955"),
},
}

for _, test := range tests {
Expand Down
2 changes: 1 addition & 1 deletion receiver/awsxrayreceiver/internal/translator/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func addHTTP(seg *awsxray.Segment, span *pdata.Span) {

if resp := seg.HTTP.Response; resp != nil {
if resp.Status != nil {
otStatus := tracetranslator.StatusCodeFromHTTP(int(*resp.Status))
otStatus := tracetranslator.StatusCodeFromHTTP(*resp.Status)
// in X-Ray exporter, the segment status is set:
// first via the span attribute, conventions.AttributeHTTPStatusCode
// then the span status. Since we are also setting the span attribute
Expand Down

0 comments on commit d5ea9e4

Please sign in to comment.