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.

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

Jump to bottom

New tracer #3836

Merged
merged 17 commits into from
Jun 25, 2020
Merged

New tracer #3836

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
fb3c5a5
Attempt to use libbeat tracer
jalvz Jun 2, 2020
3639af2
Flip the logic
jalvz Jun 3, 2020
981ecfe
Merge remote-tracking branch 'elastic/master' into new-tracer
jalvz Jun 17, 2020
1b66e0e
fix NOTICE??
jalvz Jun 18, 2020
a1f60f8
Update beats
jalvz Jun 18, 2020
448daac
Adapt to latest beats implementation
jalvz Jun 18, 2020
ab2e5a1
Merge remote-tracking branch 'elastic/master' into new-tracer
jalvz Jun 23, 2020
351a81a
Remove deprecation notice
jalvz Jun 23, 2020
df43748
Remove test
jalvz Jun 23, 2020
2944c67
Fix small bug
jalvz Jun 23, 2020
e468b8f
Code review
jalvz Jun 23, 2020
d326674
Code review
jalvz Jun 23, 2020
a716180
more cleanup
jalvz Jun 24, 2020
053abd2
Update to elastic/beats@1498938e22f3
jalvz Jun 24, 2020
43613a4
add tests
jalvz Jun 24, 2020
816a7d0
last code review
jalvz Jun 25, 2020
8840b2d
update changelog
jalvz Jun 25, 2020
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
9,856 changes: 9,494 additions & 362 deletions NOTICE.txt
View file
Open in desktop

Large diffs are not rendered by default.

71 changes: 39 additions & 32 deletions _meta/beat.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,38 +38,8 @@ apm-server:
# Url to expose expvar.
#url: "/debug/vars"

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:
# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Remote hosts to report instrumentation results to.
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s
# DEPRECATION NOTICE: the `apm-server.instrumentation` configuration block is deprecated, and has been moved to
axw marked this conversation as resolved.
Show resolved Hide resolved
# `instrumentation`, at the top level.

# A pipeline is a definition of processors applied to documents when ingesting them to Elasticsearch.
# Using pipelines involves two steps:
Expand Down Expand Up @@ -983,6 +953,43 @@ output.elasticsearch:
# Kerberos realm.
#kerberos.realm: ELASTIC

#============================= Instrumentation =============================

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:

# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Hosts to report instrumentation results to.
# For reporting to itself, leave this field commented
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s

#================================= Paths ==================================

# The home path for the apm-server installation. This is the default base path
Expand Down
71 changes: 39 additions & 32 deletions apm-server.docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,38 +38,8 @@ apm-server:
# Url to expose expvar.
#url: "/debug/vars"

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:
# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Remote hosts to report instrumentation results to.
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s
# DEPRECATION NOTICE: the `apm-server.instrumentation` configuration block is deprecated, and has been moved to
# `instrumentation`, at the top level.

# A pipeline is a definition of processors applied to documents when ingesting them to Elasticsearch.
# Using pipelines involves two steps:
Expand Down Expand Up @@ -983,6 +953,43 @@ output.elasticsearch:
# Kerberos realm.
#kerberos.realm: ELASTIC

#============================= Instrumentation =============================

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:

# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Hosts to report instrumentation results to.
# For reporting to itself, leave this field commented
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s

#================================= Paths ==================================

# The home path for the apm-server installation. This is the default base path
Expand Down
71 changes: 39 additions & 32 deletions apm-server.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,38 +38,8 @@ apm-server:
# Url to expose expvar.
#url: "/debug/vars"

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:
# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Remote hosts to report instrumentation results to.
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s
# DEPRECATION NOTICE: the `apm-server.instrumentation` configuration block is deprecated, and has been moved to
# `instrumentation`, at the top level.

# A pipeline is a definition of processors applied to documents when ingesting them to Elasticsearch.
# Using pipelines involves two steps:
Expand Down Expand Up @@ -983,6 +953,43 @@ output.elasticsearch:
# Kerberos realm.
#kerberos.realm: ELASTIC

#============================= Instrumentation =============================

# Instrumentation support for the server's HTTP endpoints and event publisher.
#instrumentation:

# Set to true to enable instrumentation of the APM Server itself.
#enabled: false

# Environment in which the APM Server is running on (eg: staging, production, etc.)
#environment: ""

# Hosts to report instrumentation results to.
# For reporting to itself, leave this field commented
#hosts:
# - http://remote-apm-server:8200

# API Key for the remote APM Server(s).
# If api_key is set then secret_token will be ignored.
#api_key:

# Secret token for the remote APM Server(s).
#secret_token:

# Enable profiling of the server, recording profile samples as events.
#
# This feature is experimental.
#profiling:
#cpu:
# Set to true to enable CPU profiling.
#enabled: false
#interval: 60s
#duration: 10s
#heap:
# Set to true to enable heap profiling.
#enabled: false
#interval: 60s

#================================= Paths ==================================

# The home path for the apm-server installation. This is the default base path
Expand Down
19 changes: 14 additions & 5 deletions beater/beater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"context"
"sync"

"github.com/elastic/beats/v7/libbeat/version"

"github.com/pkg/errors"
"go.elastic.co/apm"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -130,16 +132,23 @@ type beater struct {
// Run runs the APM Server, blocking until the beater's Stop method is called,
// or a fatal error occurs.
func (bt *beater) Run(b *beat.Beat) error {
tracer, tracerServer, err := initTracer(b.Info, bt.config, bt.logger)

tracerServer, err := newTracerServer(bt.config, b.Instrumentation.Listener())
axw marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
defer tracer.Close()

runServer := runServer
if tracerServer != nil {
runServer = runServerWithTracerServer(runServer, tracerServer, tracer)
tracer := b.Instrumentation.Tracer()
useLegacyTracer := common.MustNewVersion(version.GetDefaultVersion()).LessThan(&common.Version{Major: 8, Minor: 0})

if !tracer.Active() && useLegacyTracer {
tracer, tracerServer, err = initLegacyTracer(b.Info, bt.config, bt.logger)
Copy link
Member

Choose a reason for hiding this comment

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

Instead of holding onto that old code, can we instead dynamically rename apm-server.instrumentation to instrumentation, if the latter doesn't exist in the config?

I think we can do this by defining a ConfigOverride in libbeat/cmd/instance.Settings, whose Check method checks for existing "instrumentation", and if not specified sets it to "apm-server.instrumentation"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm how would that exactly work?

{
			Check: func(cfg *common.Config) bool {
				return cfg.HasField("instrumentation")
			},
			Config: ???

Config is just a config, not a function, so how do I copy apm-server.instrumentation?

Copy link
Member

Choose a reason for hiding this comment

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

I meant make Check do the modification. It would be a bit of a hack, so it should really be replaced with something more fit for purpose. But in the mean time...

Check: func(cfg *common.Config) bool {
    if !cfg.HasField("instrumentation") {
        // Check if cfg has "apm-server.instrumentation", and rename it to "instrumentation"
    }
    return true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, uh. that's quite a hack, yes..
ok

Copy link
Contributor

Choose a reason for hiding this comment

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

How about not using Check but changing libbeatConfigOverride to be a function and taking care of the overwrite there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But how do I pass to a function the original config I need to copy?
Or you mean to change the whole thing in libbeat?

Copy link
Member

Choose a reason for hiding this comment

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

I presume @simitt meant changing libbeat. I think that's the right long-term solution. IMO, ConditionalOverride should be an implementation of an interface like ConfigTransformer:

type ConfigTransformer interface {
    // TransformConfig transforms the provided config, returning a possibly modified config.
    TransformConfig(*common.Config) (*common.Config, error)
}

type ConditionalOverride struct {
    Check OverrideChecker
    Config *common.Config
}

func (o *ConditionalOverride) TransformConfig(in *common.Config) (*common.Config, error) {
    if !o.Check(in) {
        return in, nil
    }
    return common.MergeConfigs(in, o.Config)
}

We could then provide an alternative implementation of that interface which does what we need.

if err != nil {
return err
}
}

runServer := runServerWithTracerServer(runServer, tracerServer, tracer)
if bt.wrapRunServer != nil {
// Wrap runServer function, enabling injection of
// behaviour into the processing/reporting pipeline.
Expand Down
34 changes: 31 additions & 3 deletions beater/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import (
"testing"
"time"

"go.elastic.co/apm"

"github.com/gofrs/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -516,13 +518,39 @@ func setupServer(t *testing.T, cfg *common.Config, beatConfig *beat.BeatConfig,

// create a beat
apmBeat := &beat.Beat{
Publisher: pub,
Info: info,
Config: beatConfig,
Publisher: pub,
Info: info,
Config: beatConfig,
Instrumentation: newInstrumentation(t, info),
}
return setupBeater(t, apmBeat, baseConfig, beatConfig)
}

type instrumentation struct {
tracer *apm.Tracer
listener net.Listener
}

func newInstrumentation(t *testing.T, info beat.Info) instrumentation {
tracerServer, err := newTracerServer(config.DefaultConfig(info.Version), nil)
require.NoError(t, err)
tracer, err := apm.NewTracerOptions(apm.TracerOptions{
ServiceName: info.Beat,
ServiceVersion: info.Version,
Transport: tracerServer.transport,
})
require.NoError(t, err)
return instrumentation{tracer: tracer, listener: tracerServer.listener}
}

func (i instrumentation) Tracer() *apm.Tracer {
return i.tracer
}

func (i instrumentation) Listener() net.Listener {
return i.listener
}

var testData = func() []byte {
b, err := loader.LoadDataAsBytes("../testdata/intake-v2/transactions.ndjson")
if err != nil {
Expand Down
Loading