Wire agent config #4486

jalvz · 2020-12-02T17:01:00Z

Motivation/summary

Read configuration from Elastic Agent and use the namespace for the indices

Checklist

I have signed the Contributor License Agreement.
I have updated CHANGELOG.asciidoc

I have considered changes for:

documentation
logging (add log lines, choose appropriate log selector, etc.)
metrics and monitoring (create issue for Kibana team to add metrics to visualizations, e.g. Kibana#44001)
automated tests (add tests for the code changes, all unit tests pass locally)
telemetry
Elasticsearch Service (https://cloud.elastic.co)
Elastic Cloud Enterprise (https://www.elastic.co/products/ece)
Elastic Cloud on Kubernetes (https://www.elastic.co/elastic-cloud-kubernetes)

How to test these changes

Related issues

apmmachine · 2020-12-02T17:12:08Z

💚 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

Build Cause: Pull request #4486 updated
Start Time: 2020-12-04T09:55:30.352+0000
Duration: 36 min 1 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4621
Skipped	139
Total	4760

Steps errors

Expand to view the steps failures

`Compress`

Took 0 min 0 sec . View more details on here
Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

`Compress`

Took 0 min 0 sec . View more details on here
Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

`Test Sync`

Took 3 min 8 sec . View more details on here
Description: ./.ci/scripts/sync.sh

axw

Looks good functionally, just a few minor things I'd like to see tidied up

axw · 2020-12-03T02:55:53Z

beater/config/integration.go

+
+import "github.com/elastic/beats/v7/libbeat/common"
+
+func NewIntegrationConfig() *IntegrationConfig {


Suggested change

func NewIntegrationConfig() *IntegrationConfig {

func NewIntegrationConfig(rootConfig *common.Config) *IntegrationConfig {

And then do the unpack here, like we do in config.New?

axw · 2020-12-03T02:59:24Z

beater/beater.go

+			if integrationConfig.DataStream != nil {
+				bt.namespace = integrationConfig.DataStream.Namespace
+			}
+			if bt.namespace == "" {
+				bt.namespace = "default"
+			}


Can we encapsulate this in NewIntegrationConfig (passing in the root config), so there's always a namespace defined when NewIntegrationConfig returns? Then if other code ends up using that function, the logic is in one place.

axw · 2020-12-03T03:01:37Z

publish/pub.go

@@ -81,7 +81,7 @@ var (
 // newPublisher creates a new publisher instance.
 //MaxCPU new go-routines are started for forwarding events to libbeat.
 //Stop must be called to close the beat.Client and free resources.
-func NewPublisher(pipeline beat.Pipeline, tracer *apm.Tracer, cfg *PublisherConfig) (*Publisher, error) {
+func NewPublisher(pipeline beat.Pipeline, namespace string, tracer *apm.Tracer, cfg *PublisherConfig) (*Publisher, error) {


Add namespace as a field of PublisherConfig?

axw · 2020-12-04T02:16:36Z

beater/beater.go

}
+

If you add the following (untested), you can remove the TODO above. This addresses the issue I mentioned on Slack, where the data streams config will be defined for the initial static config, but not for the dynamic input coming from Agent:

apmServerCommonConfig.Merge(common.MustNewConfigFrom(`{"apm-server.data_streams.enabled": true}`))

Not sure I follow, when agent starts the server it does it with -E apm-server.data_streams.enabled=true, right?

codecov-io · 2020-12-04T10:10:47Z

Codecov Report

Merging #4486 (38f43fc) into master (945e483) will decrease coverage by 0.09%.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##           master    #4486      +/-   ##
==========================================
- Coverage   76.06%   75.96%   -0.10%     
==========================================
  Files         160      161       +1     
  Lines        9775     9790      +15     
==========================================
+ Hits         7435     7437       +2     
- Misses       2340     2353      +13

Impacted Files	Coverage Δ
beater/config/integration.go	`0.00% <0.00%> (ø)`
publish/pub.go	`73.80% <0.00%> (ø)`
beater/beater.go	`55.92% <14.28%> (-2.29%)`	⬇️
processor/otel/consumer.go	`93.51% <0.00%> (-0.45%)`	⬇️
kibana/connecting_client.go	`70.17% <0.00%> (+5.26%)`	⬆️

Read configuration from Elastic Agent and use the namespace for the indices

axw · 2020-12-22T04:26:10Z

Following on from #4228 (comment), I sent a transaction to APM Server and checked that a new "traces" data stream was created with the namespace "default":

GET /_data_stream/traces-*
{
  "data_streams" : [
    {
      "name" : "traces-apm-default",
      "timestamp_field" : {
        "name" : "@timestamp"
      },
      "indices" : [
        {
          "index_name" : ".ds-traces-apm-default-2020.12.22-000001",
          "index_uuid" : "3AuDXYecSVOUbw_JwGDdQw"
        }
      ],
      "generation" : 1,
      "_meta" : {
        "managed_by" : "ingest-manager",
        "managed" : true,
        "package" : {
          "name" : "apm"
        }
      },
      "status" : "YELLOW",
      "template" : "traces-apm",
      "ilm_policy" : "traces",
      "hidden" : false
    }
  ]
}

I then edited the integration to set the namespace to "nondefault", restarted elastic-agent, and send another transaction. A new data stream was created:

GET /_data_stream/traces-*
{
  "data_streams" : [
    {
      "name" : "traces-apm-default",
      "timestamp_field" : {
        "name" : "@timestamp"
      },
      "indices" : [
        {
          "index_name" : ".ds-traces-apm-default-2020.12.22-000001",
          "index_uuid" : "3AuDXYecSVOUbw_JwGDdQw"
        }
      ],
      "generation" : 1,
      "_meta" : {
        "managed_by" : "ingest-manager",
        "managed" : true,
        "package" : {
          "name" : "apm"
        }
      },
      "status" : "YELLOW",
      "template" : "traces-apm",
      "ilm_policy" : "traces",
      "hidden" : false
    },
    {
      "name" : "traces-apm-nondefault",
      "timestamp_field" : {
        "name" : "@timestamp"
      },
      "indices" : [
        {
          "index_name" : ".ds-traces-apm-nondefault-2020.12.22-000001",
          "index_uuid" : "G9c-yRmBT3KR9-MLsu9vuQ"
        }
      ],
      "generation" : 1,
      "_meta" : {
        "managed_by" : "ingest-manager",
        "managed" : true,
        "package" : {
          "name" : "apm"
        }
      },
      "status" : "YELLOW",
      "template" : "traces-apm",
      "ilm_policy" : "traces",
      "hidden" : false
    }
  ]
}

wire config coming from agent

c6ebc67

axw reviewed Dec 3, 2020

View reviewed changes

jalvz added 2 commits December 3, 2020 17:30

code review

cbd22b8

be safe

270baeb

axw approved these changes Dec 4, 2020

View reviewed changes

jalvz added 4 commits December 4, 2020 09:06

merge configs

7756212

merge configs, but right

80ce928

fix approved file

3673da9

Merge remote-tracking branch 'elastic/master' into receive-grpc-config

38f43fc

jalvz merged commit 6062235 into elastic:master Dec 4, 2020

jalvz mentioned this pull request Dec 8, 2020

Integrate with Elastic Agent #4004

Closed

15 tasks

jalvz added a commit to jalvz/apm-server that referenced this pull request Dec 15, 2020

Wire agent config (elastic#4486)

d937535

Read configuration from Elastic Agent and use the namespace for the indices

jalvz mentioned this pull request Dec 15, 2020

[7.x] Wire agent config (#4486) #4538

Merged

jalvz added a commit that referenced this pull request Dec 15, 2020

Wire agent config (#4486) (#4538)

4c687c6

Read configuration from Elastic Agent and use the namespace for the indices

simitt added test-plan v7.11.0 labels Dec 16, 2020

axw self-assigned this Dec 22, 2020

axw added the test-plan-ok label Dec 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wire agent config #4486

Wire agent config #4486

jalvz commented Dec 2, 2020

apmmachine commented Dec 2, 2020 •

edited

Loading

Build stats

Test stats 🧪

`Compress`

`Compress`

`Test Sync`

axw left a comment

axw Dec 3, 2020

axw Dec 3, 2020

axw Dec 3, 2020

axw Dec 4, 2020

jalvz Dec 4, 2020

codecov-io commented Dec 4, 2020

axw commented Dec 22, 2020


		import "github.com/elastic/beats/v7/libbeat/common"

		func NewIntegrationConfig() *IntegrationConfig {

	func NewIntegrationConfig() *IntegrationConfig {
	func NewIntegrationConfig(rootConfig common.Config) IntegrationConfig {

Wire agent config #4486

Wire agent config #4486

Conversation

jalvz commented Dec 2, 2020

Motivation/summary

Checklist

How to test these changes

Related issues

apmmachine commented Dec 2, 2020 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Steps errors

Compress

Compress

Test Sync

axw left a comment

Choose a reason for hiding this comment

axw Dec 3, 2020

Choose a reason for hiding this comment

axw Dec 3, 2020

Choose a reason for hiding this comment

axw Dec 3, 2020

Choose a reason for hiding this comment

axw Dec 4, 2020

Choose a reason for hiding this comment

jalvz Dec 4, 2020

Choose a reason for hiding this comment

codecov-io commented Dec 4, 2020

Codecov Report

axw commented Dec 22, 2020

apmmachine commented Dec 2, 2020 •

edited

Loading

`Compress`

`Compress`

`Test Sync`