Skip to content

Commit

Permalink
model: de-pointer several integer fields (#4924)
Browse files Browse the repository at this point in the history
Ports and HTTP status codes should always be
positive, so use zero as a sentinel value to
indicate omission in order to remove a pointer.
  • Loading branch information
axw authored Mar 8, 2021
1 parent 984563f commit 91aeb6a
Show file tree
Hide file tree
Showing 12 changed files with 48 additions and 53 deletions.
14 changes: 9 additions & 5 deletions model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type URL struct {
Scheme string
Full string
Domain string
Port *int
Port int
Path string
Query string
Fragment string
Expand Down Expand Up @@ -84,7 +84,7 @@ func ParseURL(original, defaultHostname, defaultScheme string) *URL {
}
if port := url.Port(); port != "" {
if intv, err := strconv.Atoi(port); err == nil {
out.Port = &intv
out.Port = intv
}
}
return out
Expand Down Expand Up @@ -132,7 +132,7 @@ type Resp struct {
}

type MinimalResp struct {
StatusCode *int
StatusCode int
Headers http.Header
TransferSize *float64
EncodedBodySize *float64
Expand All @@ -149,7 +149,9 @@ func (url *URL) Fields() common.MapStr {
fields.maybeSetString("fragment", url.Fragment)
fields.maybeSetString("domain", url.Domain)
fields.maybeSetString("path", url.Path)
fields.maybeSetIntptr("port", url.Port)
if url.Port > 0 {
fields.set("port", url.Port)
}
fields.maybeSetString("original", url.Original)
fields.maybeSetString("scheme", url.Scheme)
fields.maybeSetString("query", url.Query)
Expand Down Expand Up @@ -221,8 +223,10 @@ func (m *MinimalResp) Fields() common.MapStr {
return nil
}
var fields mapStr
if m.StatusCode > 0 {
fields.set("status_code", m.StatusCode)
}
fields.maybeSetMapStr("headers", headerToFields(m.Headers))
fields.maybeSetIntptr("status_code", m.StatusCode)
fields.maybeSetFloat64ptr("transfer_size", m.TransferSize)
fields.maybeSetFloat64ptr("encoded_body_size", m.EncodedBodySize)
fields.maybeSetFloat64ptr("decoded_body_size", m.DecodedBodySize)
Expand Down
9 changes: 3 additions & 6 deletions model/modeldecoder/rumv3/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,7 @@ func mapToResponseModel(from contextResponse, out *model.Resp) {
out.Headers = from.Headers.Val.Clone()
}
if from.StatusCode.IsSet() {
val := from.StatusCode.Val
out.StatusCode = &val
out.StatusCode = from.StatusCode.Val
}
if from.TransferSize.IsSet() {
val := from.TransferSize.Val
Expand Down Expand Up @@ -540,8 +539,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, out
destination.Address = from.Context.Destination.Address.Val
}
if from.Context.Destination.Port.IsSet() {
val := from.Context.Destination.Port.Val
destination.Port = &val
destination.Port = from.Context.Destination.Port.Val
}
out.Destination = &destination
}
Expand All @@ -564,8 +562,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, out
http.Method = from.Context.HTTP.Method.Val
}
if from.Context.HTTP.StatusCode.IsSet() {
val := from.Context.HTTP.StatusCode.Val
http.StatusCode = &val
http.StatusCode = from.Context.HTTP.StatusCode.Val
}
if from.Context.HTTP.URL.IsSet() {
http.URL = from.Context.HTTP.URL.Val
Expand Down
2 changes: 1 addition & 1 deletion model/modeldecoder/rumv3/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func TestDecodeMapToErrorModel(t *testing.T) {
var out model.Error
mapToErrorModel(&input, initializedMetadata(), time.Now(), &out)
assert.Equal(t, "https://my.site.test:9201", out.Page.URL.Full)
assert.Equal(t, 9201, *out.Page.URL.Port)
assert.Equal(t, 9201, out.Page.URL.Port)
assert.Equal(t, "https", out.Page.URL.Scheme)
})

Expand Down
2 changes: 1 addition & 1 deletion model/modeldecoder/rumv3/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func TestDecodeMapToTransactionModel(t *testing.T) {
var tr model.Transaction
mapToTransactionModel(&inputTr, initializedMetadata(), time.Now(), &tr)
assert.Equal(t, "https://my.site.test:9201", tr.Page.URL.Full)
assert.Equal(t, 9201, *tr.Page.URL.Port)
assert.Equal(t, 9201, tr.Page.URL.Port)
assert.Equal(t, "https", tr.Page.URL.Scheme)
})

Expand Down
14 changes: 5 additions & 9 deletions model/modeldecoder/v2/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ func mapToRequestURLModel(from contextRequestURL, out *model.URL) {
// should never result in an error, type is checked when decoding
port, err := strconv.Atoi(fmt.Sprint(from.Port.Val))
if err == nil {
out.Port = &port
out.Port = port
}
}
}
Expand All @@ -650,8 +650,7 @@ func mapToResponseModel(from contextResponse, out *model.Resp) {
out.HeadersSent = &val
}
if from.StatusCode.IsSet() {
val := from.StatusCode.Val
out.StatusCode = &val
out.StatusCode = from.StatusCode.Val
}
if from.TransferSize.IsSet() {
val := from.TransferSize.Val
Expand Down Expand Up @@ -772,8 +771,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, con
destination.Address = from.Context.Destination.Address.Val
}
if from.Context.Destination.Port.IsSet() {
val := from.Context.Destination.Port.Val
destination.Port = &val
destination.Port = from.Context.Destination.Port.Val
}
out.Destination = &destination
}
Expand Down Expand Up @@ -812,8 +810,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, con
response.Headers = from.Context.HTTP.Response.Headers.Val.Clone()
}
if from.Context.HTTP.Response.StatusCode.IsSet() {
val := from.Context.HTTP.Response.StatusCode.Val
response.StatusCode = &val
response.StatusCode = from.Context.HTTP.Response.StatusCode.Val
}
if from.Context.HTTP.Response.TransferSize.IsSet() {
val := from.Context.HTTP.Response.TransferSize.Val
Expand All @@ -822,8 +819,7 @@ func mapToSpanModel(from *span, metadata *model.Metadata, reqTime time.Time, con
http.Response = &response
}
if from.Context.HTTP.StatusCode.IsSet() {
val := from.Context.HTTP.StatusCode.Val
http.StatusCode = &val
http.StatusCode = from.Context.HTTP.StatusCode.Val
}
if from.Context.HTTP.URL.IsSet() {
http.URL = from.Context.HTTP.URL.Val
Expand Down
2 changes: 1 addition & 1 deletion model/modeldecoder/v2/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestDecodeMapToErrorModel(t *testing.T) {
var out model.Error
mapToErrorModel(&input, initializedMetadata(), time.Now(), modeldecoder.Config{}, &out)
assert.Equal(t, "https://my.site.test:9201", out.Page.URL.Full)
assert.Equal(t, 9201, *out.Page.URL.Port)
assert.Equal(t, 9201, out.Page.URL.Port)
assert.Equal(t, "https", out.Page.URL.Scheme)
})
}
2 changes: 1 addition & 1 deletion model/modeldecoder/v2/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ func TestDecodeMapToTransactionModel(t *testing.T) {
var out model.Transaction
mapToTransactionModel(&input, initializedMetadata(), time.Now(), modeldecoder.Config{Experimental: false}, &out)
assert.Equal(t, "https://my.site.test:9201", out.Page.URL.Full)
assert.Equal(t, 9201, *out.Page.URL.Port)
assert.Equal(t, 9201, out.Page.URL.Port)
assert.Equal(t, "https", out.Page.URL.Scheme)
})

Expand Down
16 changes: 9 additions & 7 deletions model/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ type DB struct {
// TODO(axw) combine this and "Http", which is used by transaction and error, into one type.
type HTTP struct {
URL string
StatusCode *int
StatusCode int
Method string
Response *MinimalResp
}

// Destination contains contextual data about the destination of a span, such as address and port
type Destination struct {
Address string
Port *int
Port int
}

// DestinationService contains information about the destination service of a span event
Expand Down Expand Up @@ -145,11 +145,11 @@ func (http *HTTP) fields() common.MapStr {
fields.set("url", common.MapStr(url))
}
response := http.Response.Fields()
if http.StatusCode != nil {
if http.StatusCode > 0 {
if response == nil {
response = common.MapStr{"status_code": *http.StatusCode}
} else if http.Response.StatusCode == nil {
response["status_code"] = *http.StatusCode
response = common.MapStr{"status_code": http.StatusCode}
} else if http.Response.StatusCode == 0 {
response["status_code"] = http.StatusCode
}
}
fields.maybeSetMapStr("response", response)
Expand All @@ -168,7 +168,9 @@ func (d *Destination) fields() common.MapStr {
fields.set("ip", d.Address)
}
}
fields.maybeSetIntptr("port", d.Port)
if d.Port > 0 {
fields.set("port", d.Port)
}
return common.MapStr(fields)
}

Expand Down
4 changes: 2 additions & 2 deletions model/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ func TestSpanTransform(t *testing.T) {
RUM: true,
Stacktrace: Stacktrace{{AbsPath: path}},
Labels: common.MapStr{"label_a": 12},
HTTP: &HTTP{Method: method, StatusCode: &statusCode, URL: url},
HTTP: &HTTP{Method: method, StatusCode: statusCode, URL: url},
DB: &DB{
Instance: instance,
Statement: statement,
Type: dbType,
UserName: user,
RowsAffected: &rowsAffected},
Destination: &Destination{Address: address, Port: &port},
Destination: &Destination{Address: address, Port: port},
DestinationService: &DestinationService{
Type: destServiceType,
Name: destServiceName,
Expand Down
2 changes: 1 addition & 1 deletion processor/otel/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (tx *transactionBuilder) setHTTPStatusCode(statusCode int) {
if tx.HTTP.Response == nil {
tx.HTTP.Response = &model.Resp{}
}
tx.HTTP.Response.MinimalResp.StatusCode = &statusCode
tx.HTTP.Response.MinimalResp.StatusCode = statusCode
if tx.Outcome == outcomeUnknown {
if statusCode >= 500 {
tx.Outcome = outcomeFailure
Expand Down
12 changes: 4 additions & 8 deletions processor/otel/consumer.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,8 +413,7 @@ func translateSpan(span pdata.Span, metadata model.Metadata, event *model.Span)
case pdata.AttributeValueINT:
switch kDots {
case "http.status_code":
code := int(v.IntVal())
http.StatusCode = &code
http.StatusCode = int(v.IntVal())
isHTTPSpan = true
case conventions.AttributeNetPeerPort, "peer.port":
netPeerPort = int(v.IntVal())
Expand Down Expand Up @@ -565,9 +564,9 @@ func translateSpan(span pdata.Span, metadata model.Metadata, event *model.Span)

switch {
case isHTTPSpan:
if http.StatusCode != nil {
if http.StatusCode > 0 {
if event.Outcome == outcomeUnknown {
event.Outcome = clientHTTPStatusCodeOutcome(*http.StatusCode)
event.Outcome = clientHTTPStatusCodeOutcome(http.StatusCode)
}
}
event.Type = "external"
Expand Down Expand Up @@ -595,10 +594,7 @@ func translateSpan(span pdata.Span, metadata model.Metadata, event *model.Span)
}

if destAddr != "" {
event.Destination = &model.Destination{Address: destAddr}
if destPort > 0 {
event.Destination.Port = &destPort
}
event.Destination = &model.Destination{Address: destAddr, Port: destPort}
}
if destinationService != (model.DestinationService{}) {
if destinationService.Type == "" {
Expand Down
22 changes: 11 additions & 11 deletions processor/otel/consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestHTTPTransactionURL(t *testing.T) {
Path: "/foo",
Query: "bar",
Domain: "testing.invalid",
Port: newInt(80),
Port: 80,
}, map[string]pdata.AttributeValue{
"http.scheme": pdata.NewAttributeValueString("https"),
"http.host": pdata.NewAttributeValueString("testing.invalid:80"),
Expand All @@ -131,7 +131,7 @@ func TestHTTPTransactionURL(t *testing.T) {
Path: "/foo",
Query: "bar",
Domain: "testing.invalid",
Port: newInt(80),
Port: 80,
}, map[string]pdata.AttributeValue{
"http.scheme": pdata.NewAttributeValueString("https"),
"http.server_name": pdata.NewAttributeValueString("testing.invalid"),
Expand All @@ -147,7 +147,7 @@ func TestHTTPTransactionURL(t *testing.T) {
Path: "/foo",
Query: "bar",
Domain: "testing.invalid",
Port: newInt(80),
Port: 80,
}, map[string]pdata.AttributeValue{
"http.scheme": pdata.NewAttributeValueString("https"),
"net.host.name": pdata.NewAttributeValueString("testing.invalid"),
Expand All @@ -163,7 +163,7 @@ func TestHTTPTransactionURL(t *testing.T) {
Path: "/foo",
Query: "bar",
Domain: "testing.invalid",
Port: newInt(80),
Port: 80,
}, map[string]pdata.AttributeValue{
"http.url": pdata.NewAttributeValueString("https://testing.invalid:80/foo?bar"),
})
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestHTTPSpanDestination(t *testing.T) {
t.Run("url_default_port_specified", func(t *testing.T) {
test(t, &model.Destination{
Address: "testing.invalid",
Port: newInt(443),
Port: 443,
}, &model.DestinationService{
Type: "external",
Name: "https://testing.invalid",
Expand All @@ -279,7 +279,7 @@ func TestHTTPSpanDestination(t *testing.T) {
t.Run("url_port_scheme", func(t *testing.T) {
test(t, &model.Destination{
Address: "testing.invalid",
Port: newInt(443),
Port: 443,
}, &model.DestinationService{
Type: "external",
Name: "https://testing.invalid",
Expand All @@ -291,7 +291,7 @@ func TestHTTPSpanDestination(t *testing.T) {
t.Run("url_non_default_port", func(t *testing.T) {
test(t, &model.Destination{
Address: "testing.invalid",
Port: newInt(444),
Port: 444,
}, &model.DestinationService{
Type: "external",
Name: "https://testing.invalid:444",
Expand All @@ -303,7 +303,7 @@ func TestHTTPSpanDestination(t *testing.T) {
t.Run("scheme_host_target", func(t *testing.T) {
test(t, &model.Destination{
Address: "testing.invalid",
Port: newInt(444),
Port: 444,
}, &model.DestinationService{
Type: "external",
Name: "https://testing.invalid:444",
Expand All @@ -317,7 +317,7 @@ func TestHTTPSpanDestination(t *testing.T) {
t.Run("scheme_netpeername_nethostport_target", func(t *testing.T) {
test(t, &model.Destination{
Address: "::1",
Port: newInt(444),
Port: 444,
}, &model.DestinationService{
Type: "external",
Name: "https://[::1]:444",
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestHTTPTransactionStatusCode(t *testing.T) {
tx := transformTransactionWithAttributes(t, map[string]pdata.AttributeValue{
"http.status_code": pdata.NewAttributeValueInt(200),
})
assert.Equal(t, newInt(200), tx.HTTP.Response.StatusCode)
assert.Equal(t, 200, tx.HTTP.Response.StatusCode)
}

func TestDatabaseSpan(t *testing.T) {
Expand Down Expand Up @@ -429,7 +429,7 @@ func TestDatabaseSpan(t *testing.T) {

assert.Equal(t, &model.Destination{
Address: "shopdb.example.com",
Port: newInt(3306),
Port: 3306,
}, span.Destination)

assert.Equal(t, &model.DestinationService{
Expand Down

0 comments on commit 91aeb6a

Please sign in to comment.