[On-Week] Hot update of APM/EBT labels #157093

afharo · 2023-05-08T21:32:48Z

Summary

This PR is exploring the ability to update the APM/EBT labels on the fly.

Context

During test executions and, more specifically, during Perf and Scalability test executions, we rely on APM labels to tag the transactions that belong to each tested scenario, PR, branch, and test stage (warmup vs. actual test). However, up until now, we only could update these via kibana.yml config, which implies restarting Kibana whenever anything of the tagging info needs to be updated.

This is also a problem when testing on Cloud and Serverless because restarting is even more expensive.

Alternatively, we could update the file kibana.yml and send the SIGHUP signal to Kibana. However, this solution wouldn't work on Cloud nor Serverless because we don't have access to either re-writing the kibana.yml or triggering the SIGHUP signal to the running process.

The adopted solution

We are exploring adding an HTTP API that CI can call whenever it needs to update the context. In order to do that, we need to be able to extend the configuration loaded via kibana.yml, CLI, and Env vars in a dynamic way.

This PR introduces the concept of dynamicConfiguration for plugins to opt-in for specific settings. They can be updated via PUT /internal/core/_settings. The settings in the body are validated against the dynamicConfiguration list and the actual config schemas.

In this PR, we only opt-in for telemetry.labels (validation schema).

It can be used in any of the forms:

PUT kbn:/internal/core/_settings
{
  "telemetry.labels": {
    "branch": "my-test-branch"
  }
}

PUT kbn:/internal/core/_settings
{
  "telemetry.labels.branch": "afharo-test-1"
}

PUT kbn:/internal/core/_settings
{
  "telemetry": {
    "labels": {
      "branch": "afharo-test-1"
    }
  } 
}

Why this solution?
This solution allows us to:
✅ Temporarily change certain settings: the setting overrides are only stored in memory, so restarts would wipe them.
✅ Control the settings of each individual node: similar to kibana.yml, these settings are applied to the node handling the request.
✅ Overriding config means the UI has it during bootstrap: the page-load transactions include the correct configuration because they don't require an additional HTTP request to fetch the config from another plugin/service.
✅ Immediately refreshed config

What's not that good about this solution?
Some flaws of this solution:
⚠️ Requires calling it for every node: if we test any autoscaling capabilities, we'll need to apply the settings to the new nodes as soon as they come up.
⚠️ Despite being flagged as an internal API, external users may think this is persisted and shared in some way.

👆 If we find that persistence is a requirement, we could store it as a SO (although we may miss the immediately refreshed config capability).

Alternative options considered

Exposing a custom API inside the `telemetry` plugin

The telemetry plugin could expose the API PUT /internal/telemetry/labels to update those.

✅ Separation of concerns
✅ No need to support this from core
🚫 The UI needs to call GET /api/v2/telemetry/config to fetch the dynamically changed values. This means early transactions/events like page-load might miss the dynamically-appended labels.

UI Settings

We could use Globally-Scoped UI Settings for this.

✅ They are injected into the page load
✅ They are persisted as SO and shared to all instances
✅ Good for autoscaling
✅ Good for updating through proxies
⚠️ Can't control individual tags per instance
🚫 They persist through restarts (not so good for testing multiple scenarios for the same ES instance)
🚫 They are visible in the Stack Management view
🚫 It requires plugins to handle and merge both config providers.

TODO

@dmlemeshko to validate this API is valid to be called from the CI/tests runner
@afharo to explore how to push the update to the browser
Update APM version once feat: add apm.setGlobalLabel() apm-agent-nodejs#3337 is published
Validate with the Security folks that this new HTTP API is safe to use
Validate with Kibana Core folks if we want to open this pandora box
@afharo to add more tests

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…fix'

…ibana into onweek/hot-update-of-apm-labels

…-fix'

…ibana into onweek/hot-update-of-apm-labels

…f-apm-labels

…vice

…fix'

afharo · 2023-05-16T09:10:17Z

package.json

@@ -788,7 +788,7 @@
    "deepmerge": "^4.2.2",
    "del": "^6.1.0",
    "elastic-apm-http-client": "^11.0.1",
-    "elastic-apm-node": "^3.45.0",
+    "elastic-apm-node": "git+https://github.com/afharo/apm-agent-nodejs.git#add-agent.setGlobalLabel",


This can't be merged until elastic/apm-agent-nodejs#3337 is merged and published

The PR is merged. Do we know when we're planning on publishing the version shipping the change?

@david-luna can you help us with this?

afharo · 2023-05-16T09:12:11Z

packages/core/apps/core-apps-server-internal/src/core_app.ts

+
+    router.versioned
+      .put({
+        path: '/_settings',


Using PUT /_settings to update the dynamic options. I went with this approach instead of UI Settings because we don't need persistence through restarts... Also, this allows us to set specific settings for each Kibana node in case we have multiple instances.

But happy to iterate on that and move this to UI Settings if we don't want to open this pandora box.

afharo · 2023-05-16T09:18:27Z

src/plugins/telemetry/kibana.jsonc

@@ -6,6 +6,7 @@
    "id": "telemetry",
    "server": true,
    "browser": true,
+    "enabledOnAnonymousPages": true,


this allows us to send telemetry on the login screen

Looking at the new config keys that are exposed to anon page, I wonder, Is this fine, from a security standpoint?

E.g we're now surfacing the following config props to anon pages:

'telemetry.sendUsageFrom (alternatives)', 'telemetry.sendUsageTo (any)',

Are those safe to be accessed anonymously?

I think they are safe:

sendUsageFrom can be either server or browser

sendUsageTo is either prod or staging

Ok, then I think so too yeah. I was afraid those might be urls or something

afharo · 2023-05-16T09:21:53Z

src/plugins/telemetry/public/plugin.ts

+        tap((labels) => {
+          // Hack to update the APM agent's labels.
+          // In the future we might want to expose APM as a core service to make reporting metrics much easier.
+          window.elasticApm?.addLabels(labels);


For the UI, the agent is global, so addLabels works great.

However, on the server, the nodejs agent needs the global labels setter because addLabels only applies to the current transaction.

afharo · 2023-05-16T09:24:26Z

src/plugins/telemetry/server/routes/telemetry_config.ts

+      options: {
+        authRequired: 'optional',
+      },


this allows us to fetch configuration while on the login screen.

I assume there is nothing sensitive in the response, right?

The structure of the output is declared in

kibana/src/plugins/telemetry/server/routes/telemetry_config.ts

Lines 83 to 89 in 0fcc9e0

body: schema.object({

allowChangingOptInStatus: schema.boolean(),

optIn: schema.oneOf([schema.boolean(), schema.literal(null)]),

sendUsageFrom: schema.oneOf([schema.literal('server'), schema.literal('browser')]),

telemetryNotifyUserAboutOptInDefault: schema.boolean(),

labels: labelsSchema,

}),

And our awesome tests for the versioned router will fail validation (proved) if any additional info is exposed (kudos to @jloleysens 🧡)

afharo · 2023-08-04T16:38:34Z

Also, since this functionality is currently only needed during tests, have we considered gating it with a "dev-only"-like flag (regardless of the approach we take here)?

I've hidden the feature behind a feature flag that it's only enabled on Serverless (we could decide to disable it on Serverless Prod as well if necessary). On top of that, it's an internal API, so external actors shouldn't be able to call the API.

…te-of-apm-labels

…fix'

azasypkin

Alright, I won't comment on the general functionality of coreApp.allowDynamicConfigOverrides. If it's indeed a necessity for your team, it's acceptable from a security standpoint as long as you:

✔️ Ensure that this API is hidden behind a non-default flag.
✔️ Require consumers to explicitly opt-in for configuration values that can be altered using this API.
✔️ Restrict access to this API to superusers only. While less stringent than our prior requirement of filesystem access, this is acceptable given our other implemented restrictions.
⛔ Introduce a snapshot-based test for dynamicConfig, similar to the one we have for exposeToBrowser so that we're notified whenever someone decides to utilize it.

azasypkin · 2023-08-11T12:56:32Z

src/plugins/telemetry/server/routes/telemetry_config.ts

+      options: {
+        authRequired: 'optional',
+      },


I assume there is nothing sensitive in the response, right?

…te-of-apm-labels

afharo · 2023-08-25T16:26:46Z

src/core/server/integration_tests/config/check_dynamic_config.test.ts

+  /**
+   * This test is meant to fail when any setting is flagged as capable
+   * of dynamic configuration {@link PluginConfigDescriptor.dynamicConfig}.
+   *
+   * Please, add your settings to the list with a comment of why it's required to be dynamic.
+   *
+   * The intent is to trigger a code review from the Core and Security teams to discuss potential issues.
+   */
+  test('detecting all the settings that have opted-in for dynamic in-memory updates', () => {
+    expect(getListOfDynamicConfigPaths()).toStrictEqual([
+      // We need this for enriching our Perf tests with more valuable data regarding the steps of the test
+      // Also helpful in Cloud & Serverless testing because we can't control the labels in those offerings
+      'telemetry.labels',
+    ]);
+  });


@azasypkin, I added the test you requested 😇 in the commit 2bbc917

…fix'

azasypkin

LGTM from the security point of view 👍 Thanks for adding tests!

pgayvallet

LGTM

TinaHeiligers

LGTM

dmlemeshko

Tested locally and for Cloud deployment using kibana-a-la-carte. It works as expected. Great work @afharo !

kibana-ci · 2023-08-31T11:54:03Z

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/core-apps-server-internal`	8	9	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`telemetry`	19.2KB	19.4KB	+259.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/core-apps-server-internal`	8	9	+1
`@kbn/core-plugins-server`	58	60	+2
total			+3

ESLint disabled line counts

id	before	after	diff
`@kbn/core`	8	9	+1

Total ESLint disabled count

id	before	after	diff
`@kbn/core`	9	10	+1

History

💚 Build #154428 succeeded 4e31733
💚 Build #153991 succeeded d5d42a8
💔 Build #153645 failed 730ebbc
💔 Build #153634 failed 19f114c
💔 Build #152796 failed f3fbe27

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @afharo

[On-Week] Hot update of APM/EBT labels

62c9016

afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc wg:performance Work tracked by the performance workgroup Spacetime Team:Performance labels May 8, 2023

afharo self-assigned this May 8, 2023

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

e58aa4e

…fix'

afharo mentioned this pull request May 10, 2023

feat: add apm.setGlobalLabel() elastic/apm-agent-nodejs#3337

Merged

6 tasks

afharo and others added 3 commits May 10, 2023 13:08

Use custom apm-agent-nodejs elastic/apm-agent-nodejs#3337

7644cfa

Merge branch 'onweek/hot-update-of-apm-labels' of github.com:afharo/k…

17b34e9

…ibana into onweek/hot-update-of-apm-labels

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

4b94138

…-fix'

dmlemeshko added the ci:cloud-deploy Create or update a Cloud deployment label May 10, 2023

afharo and others added 14 commits May 10, 2023 14:28

Add new method to APM mocks

da5a9b0

Merge branch 'onweek/hot-update-of-apm-labels' of github.com:afharo/k…

be56393

…ibana into onweek/hot-update-of-apm-labels

Spread the update to the browser

e031ed0

Spread initial config to the APM global labels

1c1062b

Spread the labels updates to EBT context

1cc38fb

update journey tearUp hook

4ba34c1

Allow unauthenticated requests to telemetry/config API

02b0967

Merge branch 'onweek/hot-update-of-apm-labels' of github.com:afharo/k…

b7370e0

…ibana into onweek/hot-update-of-apm-labels

add wait and move api call

dea544b

Merge remote-tracking branch 'upstream/main' into onweek/hot-update-o…

7f7fc21

…f-apm-labels

Introduce the concept of dynamic configuration in the core Config Ser…

d1adabc

…vice

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

41b9065

…fix'

Add/fix tests

464aa4f

Small improvements after self-review

d45ea1b

afharo commented May 16, 2023

View reviewed changes

afharo added 3 commits May 16, 2023 11:43

Fix ConfigService mock

7f05ea9

Update exposed configs to anonymous pages

8069850

Export missing type from the index

f956a6a

Hide behind feature-flag + add tests

bdadfab

afharo force-pushed the onweek/hot-update-of-apm-labels branch from 01c251e to bdadfab Compare August 4, 2023 16:35

afharo and others added 6 commits August 7, 2023 09:49

Merge branch 'main' of github.com:elastic/kibana into onweek/hot-upda…

d668f76

…te-of-apm-labels

Fix tests

1d58c84

Journey FTR harness to set the appropriate internal headers

666a37c

Remove accidental duplicated CLI option

f768e8c

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

97da881

…fix'

Merge branch 'main' into onweek/hot-update-of-apm-labels

51f51c9

azasypkin reviewed Aug 11, 2023

View reviewed changes

afharo added 2 commits August 23, 2023 15:59

Merge branch 'main' of github.com:elastic/kibana into onweek/hot-upda…

f3fbe27

…te-of-apm-labels

Add test to ping us when new options opt-in

2bbc917

afharo commented Aug 25, 2023

View reviewed changes

afharo and others added 6 commits August 25, 2023 18:27

Merge branch 'main' into onweek/hot-update-of-apm-labels

c6314f8

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

19f114c

…fix'

Explicit string type to help TS inferrence

730ebbc

Add missing field to the telemetry/config response schema

0fcc9e0

Merge branch 'main' into onweek/hot-update-of-apm-labels

d5d42a8

Merge branch 'main' into onweek/hot-update-of-apm-labels

4e31733

azasypkin approved these changes Aug 30, 2023

View reviewed changes

pgayvallet approved these changes Aug 30, 2023

View reviewed changes

TinaHeiligers approved these changes Aug 30, 2023

View reviewed changes

dmlemeshko approved these changes Aug 31, 2023

View reviewed changes

Merge branch 'main' into onweek/hot-update-of-apm-labels

acb62bb

afharo merged commit 0ea37c1 into elastic:main Aug 31, 2023

afharo deleted the onweek/hot-update-of-apm-labels branch August 31, 2023 12:36

kibanamachine added the v8.11.0 label Aug 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[On-Week] Hot update of APM/EBT labels #157093

[On-Week] Hot update of APM/EBT labels #157093

afharo commented May 8, 2023 •

edited

Loading

afharo May 16, 2023

pgayvallet May 22, 2023

afharo May 22, 2023

afharo May 16, 2023

afharo May 16, 2023

pgayvallet May 22, 2023 •

edited

Loading

afharo May 22, 2023

pgayvallet May 23, 2023

afharo May 16, 2023

afharo May 16, 2023

azasypkin Aug 11, 2023

afharo Aug 28, 2023 •

edited

Loading

afharo commented Aug 4, 2023

azasypkin left a comment

azasypkin Aug 11, 2023

afharo Aug 25, 2023 •

edited

Loading

azasypkin left a comment

pgayvallet left a comment

TinaHeiligers left a comment

dmlemeshko left a comment

kibana-ci commented Aug 31, 2023 •

edited

Loading

API count

ESLint disabled line counts

Total ESLint disabled count

	body: schema.object({
	allowChangingOptInStatus: schema.boolean(),
	optIn: schema.oneOf([schema.boolean(), schema.literal(null)]),
	sendUsageFrom: schema.oneOf([schema.literal('server'), schema.literal('browser')]),
	telemetryNotifyUserAboutOptInDefault: schema.boolean(),
	labels: labelsSchema,
	}),

[On-Week] Hot update of APM/EBT labels #157093

[On-Week] Hot update of APM/EBT labels #157093

Conversation

afharo commented May 8, 2023 • edited Loading

Summary

Context

The adopted solution

Alternative options considered

Exposing a custom API inside the telemetry plugin

UI Settings

TODO

Checklist

For maintainers

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet May 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afharo Aug 28, 2023 • edited Loading

Choose a reason for hiding this comment

afharo commented Aug 4, 2023

azasypkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afharo Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

azasypkin left a comment

Choose a reason for hiding this comment

pgayvallet left a comment

Choose a reason for hiding this comment

TinaHeiligers left a comment

Choose a reason for hiding this comment

dmlemeshko left a comment

Choose a reason for hiding this comment

kibana-ci commented Aug 31, 2023 • edited Loading

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

afharo commented May 8, 2023 •

edited

Loading

Exposing a custom API inside the `telemetry` plugin

pgayvallet May 22, 2023 •

edited

Loading

afharo Aug 28, 2023 •

edited

Loading

afharo Aug 25, 2023 •

edited

Loading

kibana-ci commented Aug 31, 2023 •

edited

Loading