[APM] Remove usage of internal client when fetching agent config etags metrics by sorenlouv · Pull Request #173001 · elastic/kibana

sorenlouv · 2023-12-08T23:01:48Z

Closes: #170031
Replaces: elastic/elasticsearch#101467

Problem
We need to know if an agent config has been applied at the edge (by APM agents). This is determined by comparing the etag (hash) of the config, with the etag applied at the edges.

Previously the agent config itself contained this information (config.applied_by_agent) but when running with fleet this will instead be captured in agent_config metric documents.

Currently the internal kibana user retrieves the agent_config metric documents from the APM metric index (metrics-apm-* by default). This index is configurable by the end-user so can be changed to something the internal user doesn't have access to. This is a problem.

Solution

This PR replaces the calls made by the internal client with calls made by the authenticated end user (via APMEventClient). This approach works for requests made from the browser/UI but doesn't work for background tasks made by fleet. To work around this we only conditionally query the metric index if the APMEventClient is available.
If APMEventClient is not available applied_by_agent will be undefined

ghost · 2023-12-08T23:02:05Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2023-12-09T13:47:50Z

Pinging @elastic/apm-ui (Team:APM)

elasticmachine · 2023-12-09T13:47:51Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

kibana-ci · 2023-12-11T00:56:12Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: bb39c37

Failed CI Steps

Test Failures

[job] [logs] Detection Engine - Security Solution Cypress Tests #6 / Alert user assignment - ESS & Serverless Updating assignees (single alert) removing all assignees via More actions in alerts table removing all assignees via More actions in alerts table
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with host, status and severity as the filters should redirect to alert page with host, status and severity as the filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with rule name & status as filters should redirect to alert page with rule name & status as filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with user and status as the filters should redirect to alert page with user and status as the filters
[job] [logs] Serverless Investigations - Security Solution Cypress Tests #5 / Detection response view Redirection to AlertPage should redirect to alert page with user, status and severity as the filters should redirect to alert page with user, status and severity as the filters

Metrics [docs]

✅ unchanged

History

💛 Build #182840 was flaky 36f98aad4e3c0fde989944bb0692228cd1fafc17

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

cauemarcondes

LGTM... just added small suggestions.

cauemarcondes · 2023-12-11T09:28:47Z

x-pack/plugins/apm/server/routes/settings/agent_configuration/find_exact_configuration.ts

  };
 }
+
+async function getIsAppliedByAgent({


You could probably create a simple unit test to cover this function.

This is already covered by the api test. A unit test would mock out the call to getAgentConfigEtagMetrics so it would only cover the check on agentConfiguration.applied_by_agent and appliedEtags.includes(agentConfiguration.etag. I don't think it's worth it.

If it was functionality with value in itself, then maybe, but in this case it's just an implementation detail. It might as well be inlined in findExactConfiguration instead of a separate function.

cauemarcondes · 2023-12-11T09:30:05Z

x-pack/plugins/apm/server/routes/settings/agent_configuration/list_configurations.ts

      params
    ),
-    getConfigsAppliedToAgentsThroughFleet(internalESClient, apmIndices),
+    apmEventClient ? getAgentConfigEtagMetrics(apmEventClient) : undefined,


Can't you call getIsAppliedByAgent so the logic is in one place?

No, getIsAppliedByAgent is for a single agent configuration. In this case we need to fetch every agent configuration, and map them with all available etags so we want to fetch those two in parallel.

…s metrics (elastic#173001) Closes: elastic#170031 Replaces: elastic/elasticsearch#101467 **Problem** We need to know if an agent config has been applied at the edge (by APM agents). This is determined by comparing the etag (hash) of the config, with the etag applied at the edges. Previously the agent config itself contained this information (`config.applied_by_agent`) but when running with fleet this will instead be captured in `agent_config` metric documents. Currently the internal kibana user retrieves the `agent_config` metric documents from the APM metric index (`metrics-apm-*` by default). This index is configurable by the end-user so can be changed to something the internal user doesn't have access to. This is a problem. **Solution** This PR replaces the calls made by the internal client with calls made by the authenticated end user (via `APMEventClient`). This approach works for requests made from the browser/UI but doesn't work for background tasks made by fleet. To work around this we only conditionally query the metric index if the `APMEventClient` is available. If `APMEventClient` is not available `applied_by_agent` will be `undefined` (cherry picked from commit 58c7958)

kibanamachine · 2023-12-11T11:34:07Z

💚 All backports created successfully

Status	Branch	Result
✅	8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…s metrics (elastic#173001) Closes: elastic#170031 Replaces: elastic/elasticsearch#101467 **Problem** We need to know if an agent config has been applied at the edge (by APM agents). This is determined by comparing the etag (hash) of the config, with the etag applied at the edges. Previously the agent config itself contained this information (`config.applied_by_agent`) but when running with fleet this will instead be captured in `agent_config` metric documents. Currently the internal kibana user retrieves the `agent_config` metric documents from the APM metric index (`metrics-apm-*` by default). This index is configurable by the end-user so can be changed to something the internal user doesn't have access to. This is a problem. **Solution** This PR replaces the calls made by the internal client with calls made by the authenticated end user (via `APMEventClient`). This approach works for requests made from the browser/UI but doesn't work for background tasks made by fleet. To work around this we only conditionally query the metric index if the `APMEventClient` is available. If `APMEventClient` is not available `applied_by_agent` will be `undefined` (cherry picked from commit 58c7958)

sorenlouv · 2023-12-11T22:22:25Z

💚 All backports created successfully

Status	Branch	Result
✅	8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…s metrics (elastic#173001) Closes: elastic#170031 Replaces: elastic/elasticsearch#101467 **Problem** We need to know if an agent config has been applied at the edge (by APM agents). This is determined by comparing the etag (hash) of the config, with the etag applied at the edges. Previously the agent config itself contained this information (`config.applied_by_agent`) but when running with fleet this will instead be captured in `agent_config` metric documents. Currently the internal kibana user retrieves the `agent_config` metric documents from the APM metric index (`metrics-apm-*` by default). This index is configurable by the end-user so can be changed to something the internal user doesn't have access to. This is a problem. **Solution** This PR replaces the calls made by the internal client with calls made by the authenticated end user (via `APMEventClient`). This approach works for requests made from the browser/UI but doesn't work for background tasks made by fleet. To work around this we only conditionally query the metric index if the `APMEventClient` is available. If `APMEventClient` is not available `applied_by_agent` will be `undefined` (cherry picked from commit 58c7958)

…ig etags metrics (#173001) | [APM] Unskip apm api test suite (#173055) (#173120) Backports the following to `8.12`: - #173055 - #173001

kibanamachine · 2023-12-12T18:38:00Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2023-12-13T21:48:39Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2023-12-17T21:49:07Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

sorenlouv · 2023-12-17T22:04:50Z

Backported to 8.12 in #173120

…s metrics (elastic#173001) Closes: elastic#170031 Replaces: elastic/elasticsearch#101467 **Problem** We need to know if an agent config has been applied at the edge (by APM agents). This is determined by comparing the etag (hash) of the config, with the etag applied at the edges. Previously the agent config itself contained this information (`config.applied_by_agent`) but when running with fleet this will instead be captured in `agent_config` metric documents. Currently the internal kibana user retrieves the `agent_config` metric documents from the APM metric index (`metrics-apm-*` by default). This index is configurable by the end-user so can be changed to something the internal user doesn't have access to. This is a problem. **Solution** This PR replaces the calls made by the internal client with calls made by the authenticated end user (via `APMEventClient`). This approach works for requests made from the browser/UI but doesn't work for background tasks made by fleet. To work around this we only conditionally query the metric index if the `APMEventClient` is available. If `APMEventClient` is not available `applied_by_agent` will be `undefined` (cherry picked from commit 58c7958)

sorenlouv added release_note:fix v8.13.0 v8.12.0 and removed v8.13.0 labels Dec 8, 2023

sorenlouv force-pushed the remove-usage-of-internal-client-for-agent-config-etags branch from c59fc7b to 6499c97 Compare December 8, 2023 23:49

sorenlouv added Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. labels Dec 9, 2023

sorenlouv marked this pull request as ready for review December 9, 2023 13:47

sorenlouv requested a review from a team December 9, 2023 13:47

sorenlouv force-pushed the remove-usage-of-internal-client-for-agent-config-etags branch from 6499c97 to 36f98aa Compare December 9, 2023 13:48

sorenlouv removed the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Dec 9, 2023

sorenlouv added 2 commits December 11, 2023 00:29

[APM] Remove usage of internal client when fetching agent config etags

a3ae2c3

API test changes

bb39c37

sorenlouv force-pushed the remove-usage-of-internal-client-for-agent-config-etags branch from 36f98aa to bb39c37 Compare December 10, 2023 23:29

botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label Dec 10, 2023

This was referenced Dec 11, 2023

kibana_system role missing read permissions for metrics-apm-* and logs-apm-* #170031

Closed

Add logs-apm-* and metric-apm-* read permissions for kibana_system role elastic/elasticsearch#101467

Closed

sorenlouv changed the title ~~[APM] Remove usage of internal client when fetching agent config etags~~ [APM] Remove usage of internal client when fetching agent config etags metrics Dec 11, 2023

cauemarcondes approved these changes Dec 11, 2023

View reviewed changes

sorenlouv merged commit 58c7958 into elastic:main Dec 11, 2023

sorenlouv deleted the remove-usage-of-internal-client-for-agent-config-etags branch December 11, 2023 11:28

kibanamachine added the v8.13.0 label Dec 11, 2023

kibanamachine mentioned this pull request Dec 11, 2023

[8.12] [APM] Remove usage of internal client when fetching agent config etags metrics (#173001) #173038

Closed

kibanamachine mentioned this pull request Dec 11, 2023

[APM] Unskip apm api test suite #173055

Merged

sorenlouv mentioned this pull request Dec 11, 2023

[8.12] [APM] Remove usage of internal client when fetching agent config etags metrics (#173001) | [APM] Unskip apm api test suite (#173055) #173120

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 12, 2023

sorenlouv removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 13, 2023

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2023

sorenlouv removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2023

sorenlouv added the backport:skip This PR does not require backporting label Dec 17, 2023

Conversation

sorenlouv commented Dec 8, 2023 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Dec 8, 2023

🤖 GitHub comments

Uh oh!

elasticmachine commented Dec 9, 2023

Uh oh!

elasticmachine commented Dec 9, 2023

Uh oh!

kibana-ci commented Dec 11, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

History

Uh oh!

cauemarcondes left a comment

Choose a reason for hiding this comment

Uh oh!

cauemarcondes Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

sorenlouv Dec 11, 2023 • 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!

cauemarcondes Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

sorenlouv Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 11, 2023

💚 All backports created successfully

Questions ?

Uh oh!

sorenlouv commented Dec 11, 2023

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Dec 12, 2023

Uh oh!

kibanamachine commented Dec 13, 2023

Uh oh!

kibanamachine commented Dec 17, 2023

Uh oh!

sorenlouv commented Dec 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sorenlouv commented Dec 8, 2023 •

edited by kibanamachine

Loading

sorenlouv Dec 11, 2023 •

edited

Loading