Skip to content

Enhance utility methods and get rid of normalization processor in beats. #687

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 9, 2018
Merged

Enhance utility methods and get rid of normalization processor in beats. #687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fae599f
Add checks, remove types from models.
simitt Feb 21, 2018
af34243
Move context to model and add transform method.
simitt Feb 21, 2018
514019f
Normalize events in apm-server code.
simitt Feb 21, 2018
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
2 changes: 2 additions & 0 deletions beater/pub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ func newPublisher(pipeline beat.Pipeline, N int) (*publisher, error) {
// TODO: We want to wait for events in pipeline on shutdown?
// If set >0 `Close` will block for the duration or until pipeline is empty
WaitClose: 0,

SkipNormalization: true,
})
if err != nil {
return nil, err
Expand Down
13 changes: 9 additions & 4 deletions docs/data/elasticsearch/transaction.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@
"title": "node"
},
"request": {
"body": "Hello World",
"body": {
"additional": {
"bar": 123,
"req": "additional information"
},
"str": "hello world"
},
"cookies": {
"c1": "v1",
"c2": "v2"
Expand Down Expand Up @@ -111,10 +117,9 @@
"id": "945254c5-67a5-417e-8a4e-aa29efcbfb79",
"marks": {
"navigationTiming": {
"appBeforeBootstrap": 608.9300000000001,
"appBeforeBootstrap": 608.930000,
"navigationStart": -21
},
"performance": {}
}
},
"name": "GET /api/types",
"result": "success",
Expand Down
9 changes: 8 additions & 1 deletion docs/data/intake-api/generated/transaction/payload.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,14 @@
"SERVER_SOFTWARE": "nginx",
"GATEWAY_INTERFACE": "CGI/1.1"
},
"body": "Hello World"
"body": {
"str": "hello world",
"additional": {
"foo": {},
"bar": 123,
"req": "additional information"
}
}
},
"response": {
"status_code": 200,
Expand Down
38 changes: 38 additions & 0 deletions model/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package model

import (
"github.com/elastic/apm-server/utility"
"github.com/elastic/beats/libbeat/common"
)

type Context struct {
service common.MapStr
process common.MapStr
system common.MapStr
user common.MapStr
}

func NewContext(service *Service, process *Process, system *System, user common.MapStr) *Context {
return &Context{
service: service.Transform(),
process: process.Transform(),
system: system.Transform(),
user: user,
}
}

func (c *Context) Transform(m common.MapStr) common.MapStr {
if m == nil {
m = common.MapStr{}
} else {
for k, v := range m {
Copy link
Contributor

@jalvz jalvz Feb 21, 2018

Choose a reason for hiding this comment

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

this reads weird at first sight, can you add a short comment saying that you do this to remove nulls? same for spanContext

// normalize map entries by calling utility.Add
utility.Add(m, k, v)
}
}
utility.Add(m, "service", c.service)
utility.Add(m, "process", c.process)
utility.Add(m, "system", c.system)
utility.MergeAdd(m, "user", c.user)
return m
}
103 changes: 103 additions & 0 deletions model/context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package model

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/beats/libbeat/common"
)

var ip = "127.0.0.1"

func TestContext(t *testing.T) {
tests := []struct {
process *Process
system *System
service *Service
user common.MapStr
context *Context
}{
{
process: nil,
system: nil,
service: nil,
user: nil,
context: &Context{},
},
{
process: &Process{},
system: &System{},
service: &Service{},
user: common.MapStr{},
context: &Context{
process: common.MapStr{"pid": 0},
service: common.MapStr{"name": "", "agent": common.MapStr{"version": "", "name": ""}},
system: common.MapStr{},
user: common.MapStr{},
},
},
{
process: &Process{Pid: 123},
system: &System{IP: &ip},
service: &Service{Name: "service"},
user: common.MapStr{"id": 456},
context: &Context{
process: common.MapStr{"pid": 123},
system: common.MapStr{"ip": ip},
service: common.MapStr{"name": "service", "agent": common.MapStr{"version": "", "name": ""}},
user: common.MapStr{"id": 456},
},
},
}

for idx, te := range tests {
ctx := NewContext(te.service, te.process, te.system, te.user)
assert.Equal(t, te.context, ctx,
fmt.Sprintf("<%v> Expected: %v, Actual: %v", idx, te.context, ctx))
}

}

func TestContextTransform(t *testing.T) {

tests := []struct {
context *Context
m common.MapStr
out common.MapStr
}{
{
context: &Context{},
m: common.MapStr{},
out: common.MapStr{},
},
{
context: &Context{},
m: common.MapStr{"user": common.MapStr{"id": 123}},
out: common.MapStr{"user": common.MapStr{"id": 123}},
},
{
context: &Context{
process: common.MapStr{"pid": 123},
system: common.MapStr{"ip": ip},
service: common.MapStr{"name": "service", "agent": common.MapStr{"version": "", "name": ""}},
user: common.MapStr{"id": 456},
},
m: common.MapStr{"foo": "bar", "user": common.MapStr{"id": 123, "username": "foo"}},
out: common.MapStr{
"foo": "bar",
"user": common.MapStr{"id": 456, "username": "foo"},
"process": common.MapStr{"pid": 123},
"system": common.MapStr{"ip": ip},
"service": common.MapStr{"name": "service", "agent": common.MapStr{"version": "", "name": ""}},
},
},
}

for idx, te := range tests {
out := te.context.Transform(te.m)
assert.Equal(t, te.out, out,
fmt.Sprintf("<%v> Expected: %v, Actual: %v", idx, te.out, out))
}
}
2 changes: 0 additions & 2 deletions model/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ type Process struct {
Argv []string
}

type TransformProcess func(a *Process) common.MapStr

func (p *Process) Transform() common.MapStr {
if p == nil {
return nil
Expand Down
6 changes: 0 additions & 6 deletions model/process_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ import (
"github.com/elastic/beats/libbeat/common"
)

func TestProcessTransformDefinition(t *testing.T) {
myfn := func(fn TransformProcess) string { return "ok" }
res := myfn((*Process).Transform)
assert.Equal(t, "ok", res)
}

func TestProcessTransform(t *testing.T) {
pid := 1234
processTitle := "node"
Expand Down
18 changes: 10 additions & 8 deletions model/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,22 @@ type Agent struct {
Version string
}

type TransformService func(a *Service) common.MapStr

func (s *Service) MinimalTransform() common.MapStr {
svc := common.MapStr{
"name": s.Name,
"agent": common.MapStr{
"name": s.Agent.Name,
"version": s.Agent.Version,
},
if s == nil {
return nil
}
svc := common.MapStr{"name": s.Name}
agent := common.MapStr{}
utility.Add(agent, "name", s.Agent.Name)
utility.Add(agent, "version", s.Agent.Version)
utility.Add(svc, "agent", agent)
return svc
}

func (s *Service) Transform() common.MapStr {
if s == nil {
return nil
}
svc := s.MinimalTransform()
utility.Add(svc, "version", s.Version)
utility.Add(svc, "environment", s.Environment)
Expand Down
6 changes: 0 additions & 6 deletions model/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ import (
"github.com/elastic/beats/libbeat/common"
)

func TestServiceTransformDefinition(t *testing.T) {
myfn := func(fn TransformService) string { return "ok" }
res := myfn((*Service).Transform)
assert.Equal(t, "ok", res)
}

func TestServiceTransform(t *testing.T) {

version := "5.1.3"
Expand Down
1 change: 0 additions & 1 deletion model/system.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,5 @@ func (s *System) Transform() common.MapStr {
utility.Add(system, "architecture", s.Architecture)
utility.Add(system, "platform", s.Platform)
utility.Add(system, "ip", s.IP)

return system
}
9 changes: 0 additions & 9 deletions processor/error/event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,6 @@ func (e *Event) Transform(config *pr.Config, service m.Service) common.MapStr {
return e.data
}

// This updates the event in place
func (e *Event) contextTransform(pa *payload) common.MapStr {
if e.Context == nil {
e.Context = make(map[string]interface{})
}
utility.InsertInMap(e.Context, "user", pa.User)
return e.Context
}

func (e *Event) updateCulprit(config *pr.Config) {
if config == nil || config.SmapMapper == nil {
return
Expand Down
54 changes: 3 additions & 51 deletions processor/error/package_tests/TestProcessErrorNullValues.approved.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,7 @@
"some_other_value": "foo bar"
},
"request": {
"headers": {
"content-type": null,
"cookie": null,
"user-agent": null
},
"method": "POST",
"socket": {
"encrypted": null,
"remote_address": null
},
"url": {}
},
"response": {
"headers": {
"content-type": null
}
"method": "POST"
},
"service": {
"agent": {
Expand Down Expand Up @@ -87,31 +72,8 @@
{
"@timestamp": "2017-05-09T15:04:05Z",
"context": {
"custom": null,
"request": {
"body": null,
"cookies": null,
"env": null,
"headers": null,
"http_version": null,
"method": "POST",
"socket": null,
"url": {
"full": null,
"hash": null,
"hostname": null,
"pathname": null,
"port": null,
"protocol": null,
"raw": null,
"search": null
}
},
"response": {
"finished": null,
"headers": null,
"headers_sent": null,
"status_code": null
"method": "POST"
},
"service": {
"agent": {
Expand All @@ -122,11 +84,6 @@
"name": "ruby"
},
"name": "1234_service-12a3"
},
"user": {
"email": null,
"id": null,
"username": null
}
},
"error": {
Expand All @@ -143,9 +100,6 @@
{
"@timestamp": "2017-05-09T15:04:05.999Z",
"context": {
"custom": null,
"request": null,
"response": null,
"service": {
"agent": {
"name": "ruby",
Expand All @@ -155,9 +109,7 @@
"name": "ruby"
},
"name": "1234_service-12a3"
},
"tags": null,
"user": null
}
},
"error": {
"grouping_key": "d6b3f958dfea98dc9ed2b57d5f0c48bb",
Expand Down
Loading