[APM] [experimental] Service metrics on inventory page #140868

kpatticha · 2022-09-15T23:41:03Z

~~By default the setting xpack.apm.searchAggregatedServiceMetrics is set to false .~~
Add kibana advanced setting to configure - #141147

Adds a getSearchAggregatedServiceMetrics() method which will determine, based on the date range and configuration setting whether aggregated service metrics should be used.
Adds getServiceAggregatedTransactionDetailedStatistics() query for the service metrics
Adds getServiceAggregatedTransactionStats query for the service metrics

Created separated queries for the fetching service metrics data cause the data model is quite different from aggregated transaction document (transaction.duration.histogram)

service metrics document example

{
"took": 0,
"timed_out": false,
"_shards": {
  "total": 3,
  "successful": 3,
  "skipped": 0,
  "failed": 0
},
"hits": {
  "total": {
    "value": 1,
    "relation": "eq"
  },
  "max_score": 0,
  "hits": [
    {
      "_index": ".ds-metrics-apm.internal-default-2022.09.19-000003",
      "_id": "wNGvVYMBJNnTUS6H_uDc",
      "_score": 0,
      "_source": {
        "agent": {
          "name": "go"
        },
        "data_stream.namespace": "default",
        "data_stream.type": "metrics",
        "processor": {
          "name": "metric",
          "event": "metric"
        },
        "tags": [
          "_geoip_database_unavailable_GeoLite2-City.mmdb"
        ],
        "metricset.name": "service",
        "observer": {
          "hostname": "DIXONS-JVC-TV",
          "id": "c42b6d9b-7ea6-4e65-b1fd-dd51cff84431",
          "type": "apm-server",
          "ephemeral_id": "61e84242-bfd9-4b41-8d97-89c078b96d45",
          "version": "8.5.0"
        },
        "@timestamp": "2022-09-19T12:18:00.000Z",
        "ecs": {
          "version": "1.12.0"
        },
        "service": {
          "environment": "production",
          "name": "web-go"
        },
        "data_stream.dataset": "apm.internal",
        "event": {
          "agent_id_status": "missing",
          "ingested": "2022-09-19T12:19:00Z"
        },
        "transaction": {
          "duration.summary": {
            "sum": 376492831,
            "value_count": 775
          },
          "success_count": 476,
          "failure_count": 151,
          "type": "request"
        }
      }
    }
  ]
}

To try this out, you'll need to enable aggregation in APM Server. See elastic/apm-server#8607
In order to test it locally you need to run the following:

update apm-server.yml

aggregation:
    service:
      enabled: true

run apm-server locally
a. ./apm-server -c apm-server.yml -e -d "*"
use synthrace to generate data
node scripts/synthtrace packages/elastic-apm-synthtrace/src/scenarios/high_throughput.ts --local --maxDocs 100 --apm http://localhost:8200/ --username admin --skipPackageInstall --clean

TODO:

synthrace support service metrics - [Synthrace] Update service metric aggregator to match apm-server fields #141024

github-actions · 2022-09-15T23:41:15Z

Documentation preview:

✨ Changed pages

…-ref HEAD~1..HEAD --fix'

…-fix'

…into 138527-service-metrics

elasticmachine · 2022-09-19T15:04:20Z

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

…metrics

cauemarcondes

Some small suggestions

x-pack/plugins/apm/server/lib/helpers/get_bucket_size_for_aggregated_transactions/index.ts

x-pack/plugins/apm/server/routes/services/get_services_detailed_statistics/index.ts

x-pack/plugins/apm/server/routes/services/route.ts

…-ref HEAD~1..HEAD --fix'

…into 138527-service-metrics

dgieselaar

I've left some comments, some more important than others, but generally don't wait on my approval as I'm under the weather atm and just wanted to have a quick look. Thanks for doing this!

x-pack/plugins/apm/common/elasticsearch_fieldnames.ts

x-pack/plugins/apm/server/lib/helpers/get_aggregated_metrics.ts

dgieselaar · 2022-09-20T06:51:46Z

x-pack/plugins/apm/server/lib/helpers/get_bucket_size_for_aggregated_transactions/index.ts

@@ -12,12 +12,17 @@ export function getBucketSizeForAggregatedTransactions({
  end,
  numBuckets = 50,
  searchAggregatedTransactions,
+  searchAggregatedServiceMetrics,
 }: {
  start: number;
  end: number;
  numBuckets?: number;
  searchAggregatedTransactions?: boolean;


Can we collapse this into one (maybe required) prop?

Caue left a comment on this helper as well. #140868 (comment)
We could discuss and find what's the best in a follow up PR/ticket

x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts

.../plugins/apm/server/routes/services/get_services/get_service_aggregated_transaction_stats.ts

dgieselaar · 2022-09-20T07:04:35Z

x-pack/plugins/apm/server/routes/services/get_services/get_services_items.ts

@@ -34,6 +36,7 @@ export async function getServicesItems({
  kuery: string;
  setup: ServicesItemsSetup;
  searchAggregatedTransactions: boolean;
+  searchAggregatedServiceMetrics: boolean;


I'd prefer one option here too. Not a dealbreaker, but I think it's better to make the decision (about what to search) in one place and then pass it on downwards.

@dgieselaar I think I agree. Are you suggesting to combine searchAggregatedTransactions and searchAggregatedServiceMetrics into a single option? In that case I suggest we deprecate searchAggregatedTransactions and remove searchAggregatedServiceMetrics , and create a new one like:

transactionSource: 'auto' | 'service-metrics' | 'transaction-metrics' | 'transactions'

The auto in which order should determine the source?

first 'service-metrics' then transaction-metrics?

Yes, that order. But we should only use service-metrics where it makes sense. Right now only on the inventory page.

...s/get_services_detailed_statistics/get_service_aggregated_transaction_detailed_statistics.ts

x-pack/plugins/apm/server/routes/services/get_services_detailed_statistics/index.ts

…-ref HEAD~1..HEAD --fix'

…into 138527-service-metrics

…metrics

This to ensure proper bucket counts as discovered on elastic/kibana#140868

kibana-ci · 2022-09-20T13:13:46Z

💚 Build Succeeded

Buildkite Build
Commit: e74ae86

Metrics [docs]

✅ unchanged

History

💔 Build #73846 failed 47b8e05aa241836a5525f042fc28ee26c6b2a137
💔 Build #73824 failed 0de21b7
💔 Build #73811 failed f8f356f
💔 Build #73793 failed d89a817
💚 Build #73755 succeeded 5a7475f
💛 Build #73734 was flaky 7efe002

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

docs/settings/apm-settings.asciidoc

x-pack/plugins/apm/server/lib/helpers/service_metrics/index.ts

cauemarcondes

LGTM. Just some nits that don't block the merge.

cauemarcondes · 2022-09-20T14:16:43Z

x-pack/plugins/apm/server/lib/helpers/service_metrics/index.ts

+  return [
+    {
+      term: {
+        [METRICSET_NAME]: 'service',


I feel like we could have an enum for the possible metricsset.name values.

cauemarcondes · 2022-09-20T14:19:04Z

x-pack/plugins/apm/server/lib/helpers/get_service_inventory_search_source.ts

+  kuery: string;
+}): Promise<{
+  searchAggregatedTransactions: boolean;
+  searchAggregatedServiceMetrics: boolean;


Do we have to add Metrics at the end of the name? We don't do it for transactions.

Maybe to keep it consistent?

Suggested change

searchAggregatedServiceMetrics: boolean;

searchAggregatedServices: boolean;

I also noticed this inconsistency but I think we should keep "Metrics" and get rid of "Aggregated":

Suggested change

searchAggregatedServiceMetrics: boolean;

searchTransactionsMetrics: boolean;

searchServiceMetrics: boolean;

let me address the additional feedback in a follow up PR.

Thank you all for the review!

cauemarcondes · 2022-09-20T14:22:01Z

...tes/services/get_services_detailed_statistics/get_service_transaction_detailed_statistics.ts

+            ...commonProps,
+            offset,
+          })
+        : Promise.resolve({}),


I don't think you need Promise.resolve here. isn't that enough?

Suggested change

: Promise.resolve({}),

: {},

TypeScript will complain I think

Hmm, I can't reproduce this, but I've seen issues with TypeScript no longer being able to infer the return type. Maybe it was fixed in a recent version. Runtime a plain value should be fine in any case.

x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.test.ts

gbamparop · 2022-09-20T15:40:55Z

x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts

+  failedTransactions: number | null;
+  successfulTransactions: number | null;
+}) {
+  if (isNull(failedTransactions) || failedTransactions === 0) {


This is up to you, just a preference:

Suggested change

if (isNull(failedTransactions) || failedTransactions === 0) {

if (!failedTransactions) {

dgieselaar · 2022-09-21T09:07:18Z

x-pack/plugins/apm/server/routes/services/route.ts

+
+    const { apmEventClient, config } = setup;
+    const { searchAggregatedTransactions, searchAggregatedServiceMetrics } =
+      await getServiceInventorySearchSource({


shouldn't environment be here too?

looking closer to this!

I think, if we want to be 100% accurate environment should be there, otherwise we ask if there is ANY document that matches the metricset.name: service and it might no be for the given environment.

but I'm wondering how is that different from getSearchAggregatedTransactions, why we don't include it as a filter for the aggregated transactions?

it should be there too. Not sure why it isn't, it may have been on setup before.

I can create a ticket for it unless you want to

sorenlouv · 2022-09-21T22:07:48Z

x-pack/plugins/apm/common/elasticsearch_fieldnames.ts

@@ -46,12 +46,15 @@ export const PROCESSOR_EVENT = 'processor.event';

 export const TRANSACTION_DURATION = 'transaction.duration.us';
 export const TRANSACTION_DURATION_HISTOGRAM = 'transaction.duration.histogram';
+export const TRANSACTION_DURATION_SUMMARY = 'transaction.duration.summary';


iirc we talked about splitting this file into several files, with one file per data source (eg. APM vs infra vs logs) we might also want to group the fields by doc type (transactions vs transaction metrics vs service metrics)

Service metrics on inventory page

0b2f1e9

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

e60ce8d

…-ref HEAD~1..HEAD --fix'

kpatticha force-pushed the 138527-service-metrics branch from d39f738 to 726e43d Compare September 19, 2022 08:59

kpatticha added 2 commits September 19, 2022 12:05

Create seperate query for service aggregegated transaction stats

8980a18

Fix typo and eslint errors

da16bc3

kpatticha force-pushed the 138527-service-metrics branch from 6f2d526 to da16bc3 Compare September 19, 2022 10:07

kpatticha and others added 5 commits September 19, 2022 12:27

Fix failed transaction rate function

9986942

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

36f101c

…-fix'

Use service metrics for details stats

c8e2bfa

Merge branch '138527-service-metrics' of github.com:kpatticha/kibana …

071db36

…into 138527-service-metrics

Fix throughput calculation

945b8ac

kpatticha marked this pull request as ready for review September 19, 2022 13:56

kpatticha requested a review from a team as a code owner September 19, 2022 13:56

Fix types

3da5349

botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 19, 2022

kpatticha added 2 commits September 19, 2022 17:08

Update snapshot

69af7ec

Merge branch 'main' of github.com:elastic/kibana into 138527-service-…

e961ef9

…metrics

kpatticha added release_note:enhancement v8.5.0 labels Sep 19, 2022

cauemarcondes reviewed Sep 19, 2022

View reviewed changes

kpatticha and others added 5 commits September 19, 2022 21:53

Clean up code

c865393

Create a helper to get aggregated metrics

019cabf

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

bfacacb

…-ref HEAD~1..HEAD --fix'

Add test for configuration

e6f4278

Merge branch '138527-service-metrics' of github.com:kpatticha/kibana …

7efe002

…into 138527-service-metrics

dgieselaar reviewed Sep 20, 2022

View reviewed changes

kpatticha added 2 commits September 20, 2022 09:33

Rename getAggregatedMetrics to getServiceInventorySearchSource

8cf4529

query on metricset.name

5a7475f

kpatticha requested a review from Mpdreamz September 20, 2022 09:08

kpatticha and others added 7 commits September 20, 2022 11:29

Address PR feedback

d674cc3

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

d89a817

…-ref HEAD~1..HEAD --fix'

Sort servcice metrics bucker by value_count

13611eb

Merge branch '138527-service-metrics' of github.com:kpatticha/kibana …

d93e134

…into 138527-service-metrics

Merge branch 'main' of github.com:elastic/kibana into 138527-service-…

f8f356f

…metrics

fix broken path

0de21b7

fix types

e74ae86

kpatticha force-pushed the 138527-service-metrics branch from 47b8e05 to e74ae86 Compare September 20, 2022 12:13

Mpdreamz mentioned this pull request Sep 20, 2022

Add _doc_count to Service metrics elastic/apm-server#9143

Merged

3 tasks

Mpdreamz added a commit to Mpdreamz/apm-server that referenced this pull request Sep 20, 2022

Add _doc_count to Service metrics

37514d9

This to ensure proper bucket counts as discovered on elastic/kibana#140868

gbamparop reviewed Sep 20, 2022

View reviewed changes

docs/settings/apm-settings.asciidoc Show resolved Hide resolved

gbamparop reviewed Sep 20, 2022

View reviewed changes

x-pack/plugins/apm/server/lib/helpers/service_metrics/index.ts Show resolved Hide resolved

kpatticha mentioned this pull request Sep 20, 2022

[APM] Advanced setting for enabling service metrics #141110

Closed

cauemarcondes approved these changes Sep 20, 2022

View reviewed changes

kpatticha merged commit b93dc5f into elastic:main Sep 20, 2022

kibanamachine added the backport:skip This commit does not require backporting label Sep 20, 2022

gbamparop reviewed Sep 20, 2022

View reviewed changes

x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.test.ts Show resolved Hide resolved

gbamparop reviewed Sep 20, 2022

View reviewed changes

dgieselaar reviewed Sep 21, 2022

View reviewed changes

sorenlouv reviewed Sep 21, 2022

View reviewed changes

This was referenced Sep 23, 2022

[APM] Pass additional filters to determine accurately the source #141591

Closed

[APM] Move service metric config to kibana advanced settings #141147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] [experimental] Service metrics on inventory page #140868

[APM] [experimental] Service metrics on inventory page #140868

kpatticha commented Sep 15, 2022 •

edited by dannycroft

Loading

github-actions bot commented Sep 15, 2022

elasticmachine commented Sep 19, 2022

cauemarcondes left a comment

dgieselaar left a comment

dgieselaar Sep 20, 2022

kpatticha Sep 20, 2022

dgieselaar Sep 20, 2022

sorenlouv Sep 22, 2022

kpatticha Sep 22, 2022

sorenlouv Sep 22, 2022

kibana-ci commented Sep 20, 2022

cauemarcondes left a comment

cauemarcondes Sep 20, 2022

cauemarcondes Sep 20, 2022

sorenlouv Sep 20, 2022

kpatticha Sep 21, 2022

cauemarcondes Sep 20, 2022

dgieselaar Sep 20, 2022

dgieselaar Sep 20, 2022

gbamparop Sep 20, 2022

dgieselaar Sep 21, 2022

kpatticha Sep 21, 2022

dgieselaar Sep 21, 2022

kpatticha Sep 21, 2022 •

edited

Loading

sorenlouv Sep 21, 2022 •

edited

Loading

	searchAggregatedServiceMetrics: boolean;
	searchAggregatedServices: boolean;

	searchAggregatedServiceMetrics: boolean;
	searchTransactionsMetrics: boolean;
	searchServiceMetrics: boolean;

	if (isNull(failedTransactions) \|\| failedTransactions === 0) {
	if (!failedTransactions) {

[APM] [experimental] Service metrics on inventory page #140868

[APM] [experimental] Service metrics on inventory page #140868

Conversation

kpatticha commented Sep 15, 2022 • edited by dannycroft Loading

github-actions bot commented Sep 15, 2022

elasticmachine commented Sep 19, 2022

cauemarcondes left a comment

Choose a reason for hiding this comment

dgieselaar left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Sep 20, 2022

💚 Build Succeeded

Metrics [docs]

History

cauemarcondes left a 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

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

Choose a reason for hiding this comment

kpatticha Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

sorenlouv Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

kpatticha commented Sep 15, 2022 •

edited by dannycroft

Loading

kpatticha Sep 21, 2022 •

edited

Loading

sorenlouv Sep 21, 2022 •

edited

Loading