[ACM] Agent remote configuration management - Main functionality by jalvz · Pull Request #2095 · elastic/apm-server

jalvz · 2019-04-11T14:25:41Z

Notes

Service environment is optional, and treated like any other value (i.e. a service is unequivocally identified by its name). This is the lowest common denominator, as an agent can always send an extra request for a default:

if GET config?service.name=X&service.environment=Z == 404
then GET config?service.name=X

ofc we can introduce an option to save the extra round trip.

Steps to test

Checkout [APM] Central configuration management kibana#36031, and start Elasticsearch and Kibana.
Create the CM index with defined mapping:

PUT .apm-cm
{
  "mappings": {
    "properties": {
      "service": {
        "properties": {
          "environment": {
            "type": "keyword",
            "ignore_above": 1024
          },
          "name": {
            "type": "keyword",
            "ignore_above": 1024
          }
        }
      },
      "settings": {
        "properties": {
          "sample_rate": {
            "type": "float"
          }
        }
      },
      "timestamp": {
        "properties": {
          "us": {
            "type": "long"
          }
        }
      }
    }
  }
}

Index some config docs


PUT .apm-cm/_doc/1
{
    "service": { 
      "name": "opbeans-lisp",
      "environment": "lambda"
    },
    "@timestamp" : "2019-03-01T12:12:12",
    "settings": {
      "sampling_rate": 0.1
    }
}

PUT .apm-cm/_doc/2
{
    "service": { 
      "name": "opbeans-fortran",
      "environment": "rocket-launchpad"
    },
    "@timestamp" : "2019-04-01T12:12:12",
    "settings": {
      "sampling_rate": 0.4
    }
}

PUT .apm-cm/_doc/3
{
    "service": { 
      "name": "opbeans-fortran",
      "environment": "rocket-launchpad"
    },
    "@timestamp" : "2019-07-01T12:12:12",
    "settings": {
      "sampling_rate": 0.7
    }
}

PUT .apm-cm/_doc/4
{
    "service": { 
      "name": "opbeans-fortran",
      "environment": "testbed"
    },
    "@timestamp" : "2019-10-01T12:12:12",
    "settings": {
      "sampling_rate": 1.0
    }
}

PUT .apm-cm/_doc/5
{
    "service": { 
      "name": "opbeans-fortran"
    },
    "@timestamp" : "2019-12-01T12:12:12",
    "settings": {
      "sampling_rate": 0.12
    }
}

Alternatively, create a config directly in the UI by visiting /app/apm#/settings/new.
You should have created some transactions first.

Start APM Server with -E apm-server.kibana.enabled -E apm-server.kibana.path properties set and send requests to the config endpoint:
curl http://localhost:8200/config?service.name=apm-agent-js

{
  "sampling_rate": "0.4"
}

jalvz · 2019-04-11T14:30:57Z

make check-full fails with "struct field tag json:settings not compatible with reflect.StructTag.Get: bad syntax for struct tag value", which I have no clue what is about.

branch compiles and runs fine otherwise

jalvz · 2019-04-11T14:31:26Z

@simitt @graphaelli it would be great if you can have a look

graphaelli · 2019-04-16T20:45:29Z

Took a look and the api and response make sense to me

jalvz · 2019-06-04T14:59:59Z

@graphaelli is there something missing/wrong here preventing merge, other than golint errors?

graphaelli · 2019-06-05T21:33:07Z

I expected we were going to settle on the storage format before merging

jalvz · 2019-06-06T08:10:07Z

Worst case I would just have to remove this function: https://github.com/elastic/apm-server/pull/2095/files#diff-b6b7a562b4bbb8af1d07c8eaf1fda8b0R41, which is trivial (revert 5d3a467). Or am I missing something else?

I'd really avoid PRs open for more than 1 month if possible

simitt

Great work @jalvz ! I left a couple of comments, most of them are nits.

_meta/beat.yml

agentcfg/fetch.go

beater/agent_config_handler.go

beater/beater.go

beater/route_config.go

beater/agent_config_handler.go

simitt · 2019-06-14T09:43:59Z

agentcfg/fetch.go

+)
+
+var (
+	endpoint   = "/api/apm/settings/cm/search"


nit: this could be a const

agentcfg/model.go

simitt · 2019-06-14T10:34:36Z

beater/config.go

 		},
-		Mode: ModeProduction,
+		Mode:   ModeProduction,
+		Kibana: nil,


Why are you setting a nil default?

Because a default of common.Config() would imply enabled=true.
That means that after upgrading, users would be welcome with a new requirement (Kibana) or an error in the logs.

Ok. I'd be more explicit about it by either setting common.NewConfigFrom(`enabled: false`) or adding a dedicated test to the config_test.go checking that Kibana.Enabled() == false. Otherwise it could easily happen that this gets removed in the future without anything failing.

roncohen · 2019-06-14T11:56:25Z

_meta/beat.yml

  #ilm:
    #enabled: false

+  #kibana:


has there been a discussion around using the existing kibana settings instead of introducing a new one?

Yes, it has been discussed.

This is the same setting, just in a different namespace.

The beats setting is under setup which is not correct in our case; this is not for a setup command.

Also, should we use that, the server would refuse to start if Kibana is not reachable, which is not what we want, and not coherent eg. with Elasticsearch output.

Gotcha 👍

simitt · 2019-06-17T06:36:36Z

beater/config.go

 		},
-		Mode: ModeProduction,
+		Mode:   ModeProduction,
+		Kibana: nil,


Ok. I'd be more explicit about it by either setting common.NewConfigFrom(`enabled: false`) or adding a dedicated test to the config_test.go checking that Kibana.Enabled() == false. Otherwise it could easily happen that this gets removed in the future without anything failing.

simitt · 2019-06-17T06:37:33Z

vendor/github.com/elastic/beats/libbeat/kibana/client.go

@@ -176,15 +176,14 @@ func (conn *Connection) Request(method, extraPath string,
 	return resp.StatusCode, result, retError


Please remove this file before merging.

This introduces a new requirement (Kibana) and setting for it, in the apm-server namespace (disabled by default). It also adds an endpoint /v1/agent/configs for agents to poll configuration, requiring them to be authenticated (except RUM). Supersedes elastic#2095 More info in elastic/apm#4

jalvz · 2019-06-17T09:28:19Z

Superseded by #2289

This introduces a new requirement (Kibana) and setting for it, in the apm-server namespace (disabled by default). It also adds an endpoint /v1/agent/configs for agents to poll configuration, requiring them to be authenticated (except RUM). Supersedes #2095 More info in elastic/apm#4

This introduces a new requirement (Kibana) and setting for it, in the apm-server namespace (disabled by default). It also adds an endpoint /v1/agent/configs for agents to poll configuration, requiring them to be authenticated (except RUM). Supersedes elastic#2095 More info in elastic/apm#4

This introduces a new requirement (Kibana) and setting for it, in the apm-server namespace (disabled by default). It also adds an endpoint /v1/agent/configs for agents to poll configuration, requiring them to be authenticated (except RUM). Supersedes #2095 More info in elastic/apm#4

jalvz self-assigned this Apr 11, 2019

jalvz added the [zube]: In Progress label Apr 11, 2019

sorenlouv mentioned this pull request Apr 12, 2019

[APM] Central Configuration Management for agents in APM UI elastic/kibana#34990

Closed

jalvz mentioned this pull request Apr 18, 2019

Proxy configuration from Kibana to agents #2126

Closed

5 tasks

jalvz force-pushed the cm-poc branch 3 times, most recently from ba0cd64 to 3900229 Compare May 24, 2019 11:37

jalvz changed the title ~~Proof of concept for agent remote configuration~~ Agent remote configuration main functionality May 24, 2019

jalvz changed the title ~~Agent remote configuration main functionality~~ [ACM] Agent remote configuration management - Main functionality May 24, 2019

This was referenced May 24, 2019

[ACM] Add Integration tests #2219

Closed

[ACM] Optimization / caching #2220

Closed

jalvz added the feature label May 24, 2019

jalvz added this to the 7.3 milestone May 24, 2019

jalvz added [zube]: In Review and removed [zube]: In Progress labels May 24, 2019

jalvz force-pushed the cm-poc branch from 3900229 to e3e021d Compare May 24, 2019 12:02

jalvz mentioned this pull request May 28, 2019

Remote config elastic/apm-agent-java#640

Merged

jalvz force-pushed the cm-poc branch 5 times, most recently from aa792c2 to fb27508 Compare June 4, 2019 14:48

jalvz added 2 commits June 6, 2019 10:42

Proof of concept for agent remote configuration

5a61845

use Kibana config in apm-server namespace

64337b6

jalvz added 5 commits June 6, 2019 10:45

override relevant vendored code with next beats update content

be43167

remove the top level metatada attribute

978af63

return everything as string

5d3a467

add unit tests

3e163b2

make the linter happy

a1ad347

jalvz force-pushed the cm-poc branch from fb27508 to a1ad347 Compare June 6, 2019 09:08

simitt reviewed Jun 7, 2019

View reviewed changes

jalvz added 3 commits June 7, 2019 14:02

code review comments

4ce5d0a

change endpoint name (code review)

c097fc9

Merge remote-tracking branch 'elastic/master' into cm-poc

58f7533

simitt reviewed Jun 14, 2019

View reviewed changes

jalvz added 2 commits June 14, 2019 12:46

Merge remote-tracking branch 'elastic/master' into cm-poc

c1f7a00

code review comments

b3fc426

roncohen reviewed Jun 14, 2019

View reviewed changes

simitt approved these changes Jun 17, 2019

View reviewed changes

jalvz added 3 commits June 17, 2019 09:53

more code reviews

8ee337f

Merge remote-tracking branch 'elastic/master' into cm-poc

475d0bf

fix tests

93f5a86

jalvz mentioned this pull request Jun 17, 2019

Adds initial support for agent remote configuration #2289

Merged

jalvz closed this Jun 17, 2019

zube bot added [zube]: Done and removed [zube]: In Review labels Jun 17, 2019

graphaelli removed the [zube]: Done label Jun 25, 2019

watson mentioned this pull request Jul 4, 2019

Agent remote configuration elastic/apm#76

Closed

7 tasks

		@@ -176,15 +176,14 @@ func (conn *Connection) Request(method, extraPath string,
		return resp.StatusCode, result, retError

Conversation

jalvz commented Apr 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Steps to test

Uh oh!

jalvz commented Apr 11, 2019

Uh oh!

jalvz commented Apr 11, 2019

Uh oh!

graphaelli commented Apr 16, 2019

Uh oh!

jalvz commented Jun 4, 2019

Uh oh!

graphaelli commented Jun 5, 2019

Uh oh!

jalvz commented Jun 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jalvz Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jalvz commented Jun 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jalvz commented Apr 11, 2019 •

edited

Loading

jalvz commented Jun 6, 2019 •

edited

Loading

jalvz Jun 14, 2019 •

edited

Loading