Remove dependencies from non-beater code on beater/config #8929

axw · 2022-08-23T03:26:17Z

Motivation/summary

Stop depending directly on beater/config from all the packages. Instead, limit use of beater/config to beater itself, and translate configuration into just the bits needed by each downstream package. This will enable us to make changes to configuration independently of updating downstream users.

This does lead to a bit of duplication, but as Rob says, "a little copying is better than a little dependency".

Checklist

~~- [ ] Update CHANGELOG.asciidoc~~
~~- [ ] Update package changelog.yml (only if changes to apmpackage have been made)~~
~~- [ ] Documentation has been updated~~

How to test these changes

N/A

Related issues

#8928

Duplicate the configuration in agentcfg, translating to it from the beater/config struct in beater setup code.

Use kibana.ClientConfig directly. That type has grown an APIKey field since we defined ours.

Consruct fleet-server base URLs in the beater package, and duplicate the source map Fleet Artifact reference structure in the sourcemap package.

Introduce stream.Config, which is passed into the various Processor constructors. This is populated by beater.

mergify · 2022-08-23T03:26:31Z

This pull request does not have a backport label. Could you fix it @axw? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-7.x is the label to automatically backport to the 7.x branch.
backport-7./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

apmmachine · 2022-08-23T03:50:59Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-08-23T07:04:25.924+0000
Duration: 28 min 36 sec

Test stats 🧪

Test	Results
Failed	0
Passed	129
Skipped	0
Total	129

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate and publish the docker images.
/test windows : Build & tests on Windows.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

apmmachine · 2022-08-23T03:51:03Z

📚 Go benchmark report

Diff with the main branch

name                                                 old time/op    new time/op    delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
BackendProcessor-12                                    0.00ns ±88%    0.00ns ±85%  -71.43%  (p=0.008 n=5+5)
RUMV3Processor-12                                      0.00ns ±27%    0.00ns ±53%  -61.40%  (p=0.008 n=5+5)
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
AggregateTransaction-12                                82.1ns ± 1%    83.1ns ± 1%   +1.23%  (p=0.016 n=5+5)
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64
ShardedWriteTransactionUncontended-12                   916ns ± 6%     833ns ± 7%   -9.16%  (p=0.008 n=5+5)
IsTraceSampled/unknown-12                               367ns ± 1%     374ns ± 2%   +2.05%  (p=0.016 n=5+5)

name                                                 old alloc/op   new alloc/op   delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64

name                                                 old allocs/op  new allocs/op  delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64

report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat

marclop

Looks good! I too prefer de-coupling configs, I think that in practice it'll make little or no difference to be copying around the configuration since most of the copying will happen just once.

marclop · 2022-08-23T05:10:00Z

internal/beater/api/mux_config_agent_test.go

@@ -38,18 +37,22 @@ func TestConfigAgentHandler_AuthorizationMiddleware(t *testing.T) {
 		rec, err := requestToMuxerWithPattern(cfg, AgentConfigPath)
 		require.NoError(t, err)
 		require.Equal(t, http.StatusUnauthorized, rec.Code)
-		approvaltest.ApproveJSON(t, approvalPathConfigAgent(t.Name()), rec.Body.Bytes())
+		assert.JSONEq(t,


lahsivjar

Love the beater refactoring ❤️

axw added 4 commits August 23, 2022 11:09

agentcfg: decouple from beater/config

f032e88

Duplicate the configuration in agentcfg, translating to it from the beater/config struct in beater setup code.

kibana: decouple from beater/config

ff6902f

Use kibana.ClientConfig directly. That type has grown an APIKey field since we defined ours.

sourcemap: decouple from beater/config

2c93184

Consruct fleet-server base URLs in the beater package, and duplicate the source map Fleet Artifact reference structure in the sourcemap package.

processor/stream: decouple from beater/config

cca5417

Introduce stream.Config, which is passed into the various Processor constructors. This is populated by beater.

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Aug 23, 2022

axw marked this pull request as ready for review August 23, 2022 04:17

axw requested a review from a team August 23, 2022 04:17

marclop approved these changes Aug 23, 2022

View reviewed changes

Merge branch 'main' into beater-config-isolation

7e3a3a8

axw enabled auto-merge (squash) August 23, 2022 05:28

lahsivjar approved these changes Aug 23, 2022

View reviewed changes

Merge branch 'main' into beater-config-isolation

e8af53e

axw merged commit f29f76c into elastic:main Aug 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove dependencies from non-beater code on beater/config #8929

Remove dependencies from non-beater code on beater/config #8929

axw commented Aug 23, 2022

mergify bot commented Aug 23, 2022

apmmachine commented Aug 23, 2022 •

edited

Loading

Build stats

Test stats 🧪

apmmachine commented Aug 23, 2022 •

edited

Loading

marclop left a comment

marclop Aug 23, 2022

lahsivjar left a comment

Remove dependencies from non-beater code on beater/config #8929

Remove dependencies from non-beater code on beater/config #8929

Conversation

axw commented Aug 23, 2022

Motivation/summary

Checklist

How to test these changes

Related issues

mergify bot commented Aug 23, 2022

apmmachine commented Aug 23, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

apmmachine commented Aug 23, 2022 • edited Loading

📚 Go benchmark report

marclop left a comment

Choose a reason for hiding this comment

marclop Aug 23, 2022

Choose a reason for hiding this comment

lahsivjar left a comment

Choose a reason for hiding this comment

apmmachine commented Aug 23, 2022 •

edited

Loading

apmmachine commented Aug 23, 2022 •

edited

Loading