Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Additional data telemetry changes #71112

Merged
merged 16 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,877 changes: 952 additions & 925 deletions x-pack/plugins/apm/common/__snapshots__/apm_telemetry.test.ts.snap

Large diffs are not rendered by default.

59 changes: 27 additions & 32 deletions x-pack/plugins/apm/common/apm_telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,43 @@
* you may not use this file except in compliance with the Elastic License.
*/

import {
getApmTelemetryMapping,
mergeApmTelemetryMapping,
} from './apm_telemetry';
import { getApmTelemetryMapping } from './apm_telemetry';

// Add this snapshot serializer for this test. The default snapshot serializer
// prints "Object" next to objects in the JSON output, but we want to be able to
// Use the output from this JSON snapshot to share with the telemetry team. When
// new fields are added to the mapping, we'll have a diff in the snapshot.
expect.addSnapshotSerializer({
print: (contents) => {
return JSON.stringify(contents, null, 2);
},
test: () => true,
});

describe('APM telemetry helpers', () => {
describe('getApmTelemetry', () => {
describe('getApmTelemetryFullPath', () => {
// This test creates a snapshot with the JSON of our full telemetry mapping
// that can be PUT in a query to the index on the telemetry cluster. Sharing
// the contents of the snapshot with the telemetry team can provide them with
// useful information about changes to our telmetry.
it('generates a JSON object with the telemetry mapping', () => {
expect(getApmTelemetryMapping()).toMatchSnapshot();
});
});

describe('mergeApmTelemetryMapping', () => {
describe('with an invalid mapping', () => {
it('throws an error', () => {
expect(() => mergeApmTelemetryMapping({})).toThrowError();
});
});

describe('with a valid mapping', () => {
it('merges the mapping', () => {
// This is "valid" in the sense that it has all of the deep fields
// needed to merge. It's not a valid mapping opbject.
const validTelemetryMapping = {
mappings: {
expect({
properties: {
stack_stats: {
properties: {
stack_stats: {
kibana: {
properties: {
kibana: {
properties: { plugins: { properties: { apm: {} } } },
plugins: {
properties: {
apm: getApmTelemetryMapping(),
},
},
},
},
},
},
};

expect(
mergeApmTelemetryMapping(validTelemetryMapping)?.mappings.properties
.stack_stats.properties.kibana.properties.plugins.properties.apm
).toEqual(getApmTelemetryMapping());
});
},
}).toMatchSnapshot();
});
});
});
15 changes: 1 addition & 14 deletions x-pack/plugins/apm/common/apm_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { produce } from 'immer';
import { AGENT_NAMES } from './agent_name';

/**
Expand Down Expand Up @@ -199,6 +198,7 @@ export function getApmTelemetryMapping() {
agent_configuration: tookProperties,
agents: tookProperties,
cardinality: tookProperties,
cloud: tookProperties,
groupings: tookProperties,
indices_stats: tookProperties,
integrations: tookProperties,
Expand All @@ -221,16 +221,3 @@ export function getApmTelemetryMapping() {
},
};
}

/**
* Merge a telemetry mapping object (from https://github.com/elastic/telemetry/blob/master/config/templates/xpack-phone-home.json)
* with the output from `getApmTelemetryMapping`.
*/
export function mergeApmTelemetryMapping(
xpackPhoneHomeMapping: Record<string, any>
) {
return produce(xpackPhoneHomeMapping, (draft: Record<string, any>) => {
draft.mappings.properties.stack_stats.properties.kibana.properties.plugins.properties.apm = getApmTelemetryMapping();
return draft;
});
}
10 changes: 2 additions & 8 deletions x-pack/plugins/apm/dev_docs/telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,9 @@ The mapping for the telemetry data is here under `stack_stats.kibana.plugins.apm

The mapping used there can be generated with the output of the [`getTelemetryMapping`](../common/apm_telemetry.ts) function.

To make a change to the mapping, edit this function, run the tests to update the snapshots, then use the `merge_telemetry_mapping` script to merge the data into the telemetry repository.
The `schema` property of the `makeUsageCollector` call in the [`createApmTelemetry` function](../server/lib/apm_telemetry/index.ts) contains the output of `getTelemetryMapping`. Pull requests with changes to the schema should automatically notify the Telemetry team so they can update the mapping in the telemetry clusters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: In order for the Kibana Telemetry team to be notified, devs need to run node scripts/telemetry_check.js --fix. It will update the file containing all the schemas (and owned by the Telemetry team).

At the moment, APM is excluded so it doesn't fail (

"plugins/apm/server/lib/apm_telemetry/index.ts",
) and listed in this issue as pending to be fixed: #70180

I tried with removing the exclusion but it fails.
For progress, I'd say we merge this PR and tackle this issue separately 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the reason it's failing is because file with the usage collector call is parsed directly so the whole schema has to be there as a literal, whereas we're calling a function schema: getApmTelemetryMapping(), which it fails to parse as a valid schema.

Copy link
Contributor

@TinaHeiligers TinaHeiligers Jul 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smith there are two other issues I've found thus far while trying to resolve the telemetry-check failure:

  1. The schema needs to be what's displayed in the doc (as types, not actual values) and that the usage collector needs to be explicitly typed.
Here's the schema I got from your mapping
export const apmTelemetrySchema = {
  agents: {
    dotnet: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    go: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    java: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    'js-base': {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    nodejs: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    python: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    ruby: {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
    'rum-js': {
      agent: {
        version: { type: 'keyword' },
      },
      service: {
        framework: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        language: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
        runtime: {
          composite: { type: 'keyword' },
          name: { type: 'keyword' },
          version: { type: 'keyword' },
        },
      },
    },
  },
  cloud: {
    availability_zone: { type: 'keyword' },
    provider: { type: 'keyword' },
    region: { type: 'keyword' },
  },
  counts: {
    agent_configuration: {
      all: { type: 'long' },
    },
    error: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
    max_error_groups_per_service: {
      '1d': { type: 'long' },
    },
    max_transaction_groups_per_service: {
      '1d': { type: 'long' },
    },
    metric: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
    onboarding: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
    services: {
      '1d': { type: 'long' },
    },
    sourcemap: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
    span: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
    traces: {
      '1d': { type: 'long' },
    },
    transaction: {
      '1d': { type: 'long' },
      all: { type: 'long' },
    },
  },
  cardinality: {
    user_agent: {
      original: {
        all_agents: {
          '1d': { type: 'long' },
        },
        rum: {
          '1d': { type: 'long' },
        },
      },
    },
    transaction: {
      name: {
        all_agents: {
          '1d': { type: 'long' },
        },
        rum: {
          '1d': { type: 'long' },
        },
      },
    },
  },
  has_any_services: {
    type: 'boolean',
  },
  indices: {
    all: {
      total: {
        docs: { count: { type: 'long' } },
        store: { size_in_bytes: { type: 'long' } },
      },
    },
    shards: { total: { type: 'long' } },
  },
  integrations: {
    ml: { all_jobs_count: { type: 'long' } },
  },
  retainment: {
    error: { ms: { type: 'long' } },
    metric: { ms: { type: 'long' } },
    onboarding: { ms: { type: 'long' } },
    span: { ms: { type: 'long' } },
    transaction: { ms: { type: 'long' } },
  },
  services_per_agent: {
    dotnet: { type: 'long' },
    go: { type: 'long' },
    java: { type: 'long' },
    'js-base': { type: 'long' },
    nodejs: { type: 'long' },
    python: { type: 'long' },
    ruby: { type: 'long' },
    'rum-js': { type: 'long' },
  },
  tasks: {
    agent_configuration: { took: { ms: { type: 'long' } } },
    agents: { took: { ms: { type: 'long' } } },
    cardinality: { took: { ms: { type: 'long' } } },
    cloud: { took: { ms: { type: 'long' } } },
    groupings: { took: { ms: { type: 'long' } } },
    indices_stats: { took: { ms: { type: 'long' } } },
    integrations: { took: { ms: { type: 'long' } } },
    processor_events: { took: { ms: { type: 'long' } } },
    services: { took: { ms: { type: 'long' } } },
    versions: { took: { ms: { type: 'long' } } },
  },
  version: {
    apm_server: {
      major: { type: 'long' },
      minor: { type: 'long' },
      patch: { type: 'long' },
    },
  },
};

(note: there are a few improvements we also need to make to the allowed schema types, including allowing ignore_above for keyword fields)
2. The usageCollector is complaining of a missing type, even though type: "apm" is there. I think it's because the collector isn't typed explicitly but we'll figure all these issues out in #70180


If the [telemetry repository](https://github.com/elastic/telemetry) is cloned as a sibling to the kibana directory, you can run the following from x-pack/plugins/apm:

```bash
node ./scripts/merge-telemetry-mapping.js ../../../../telemetry/config/templates/xpack-phone-home.json
```

this will replace the contents of the mapping in the repository checkout with the updated mapping. You can then [follow the telemetry team's instructions](https://github.com/elastic/telemetry#mappings) for opening a pull request with the mapping changes.
When adding a task, the key of the task and the `took` properties need to be added under the `tasks` properties in the mapping, as when tasks run they report the time they took.

The queries for the stats are in the [collect data telemetry tasks](../server/lib/apm_telemetry/collect_data_telemetry/tasks.ts).

Expand Down
21 changes: 0 additions & 21 deletions x-pack/plugins/apm/scripts/merge-telemetry-mapping.js

This file was deleted.

30 changes: 0 additions & 30 deletions x-pack/plugins/apm/scripts/merge-telemetry-mapping/index.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('data telemetry collection tasks', () => {
} as ApmIndicesConfig;

describe('cloud', () => {
const cloudTask = tasks.find((task) => task.name === 'cloud');
const task = tasks.find((t) => t.name === 'cloud');

it('returns a map of cloud provider data', async () => {
const search = jest.fn().mockResolvedValueOnce({
Expand All @@ -42,7 +42,7 @@ describe('data telemetry collection tasks', () => {
},
});

expect(await cloudTask?.executor({ indices, search } as any)).toEqual({
expect(await task?.executor({ indices, search } as any)).toEqual({
cloud: {
availability_zone: ['us-west-1', 'europe-west1-c'],
provider: ['aws', 'gcp'],
Expand All @@ -55,7 +55,7 @@ describe('data telemetry collection tasks', () => {
it('returns an empty map', async () => {
const search = jest.fn().mockResolvedValueOnce({});

expect(await cloudTask?.executor({ indices, search } as any)).toEqual({
expect(await task?.executor({ indices, search } as any)).toEqual({
cloud: {
availability_zone: [],
provider: [],
Expand All @@ -66,16 +66,91 @@ describe('data telemetry collection tasks', () => {
});
});

describe('processor_events', () => {
const task = tasks.find((t) => t.name === 'processor_events');

it('returns a map of processor events', async () => {
const getTime = jest
.spyOn(Date.prototype, 'getTime')
.mockReturnValue(1594330792957);

const search = jest.fn().mockImplementation((params: any) => {
const isTotalHitsQuery = params?.body?.track_total_hits;

return Promise.resolve(
isTotalHitsQuery
? { hits: { total: { value: 1 } } }
: {
hits: {
hits: [{ _source: { '@timestamp': 1 } }],
},
}
);
});

expect(await task?.executor({ indices, search } as any)).toEqual({
counts: {
error: {
'1d': 1,
all: 1,
},
metric: {
'1d': 1,
all: 1,
},
onboarding: {
'1d': 1,
all: 1,
},
sourcemap: {
'1d': 1,
all: 1,
},
span: {
'1d': 1,
all: 1,
},
transaction: {
'1d': 1,
all: 1,
},
},
retainment: {
error: {
ms: 0,
},
metric: {
ms: 0,
},
onboarding: {
ms: 0,
},
sourcemap: {
ms: 0,
},
span: {
ms: 0,
},
transaction: {
ms: 0,
},
},
});

getTime.mockRestore();
});
});

describe('integrations', () => {
const integrationsTask = tasks.find((task) => task.name === 'integrations');
const task = tasks.find((t) => t.name === 'integrations');

it('returns the count of ML jobs', async () => {
const transportRequest = jest
.fn()
.mockResolvedValueOnce({ body: { count: 1 } });

expect(
await integrationsTask?.executor({ indices, transportRequest } as any)
await task?.executor({ indices, transportRequest } as any)
).toEqual({
integrations: {
ml: {
Expand All @@ -90,7 +165,7 @@ describe('data telemetry collection tasks', () => {
const transportRequest = jest.fn().mockResolvedValueOnce({});

expect(
await integrationsTask?.executor({ indices, transportRequest } as any)
await task?.executor({ indices, transportRequest } as any)
).toEqual({
integrations: {
ml: {
Expand All @@ -101,4 +176,57 @@ describe('data telemetry collection tasks', () => {
});
});
});

describe('indices_stats', () => {
const task = tasks.find((t) => t.name === 'indices_stats');

it('returns a map of index stats', async () => {
const indicesStats = jest.fn().mockResolvedValueOnce({
_all: { total: { docs: { count: 1 }, store: { size_in_bytes: 1 } } },
_shards: { total: 1 },
});

expect(await task?.executor({ indices, indicesStats } as any)).toEqual({
indices: {
shards: {
total: 1,
},
all: {
total: {
docs: {
count: 1,
},
store: {
size_in_bytes: 1,
},
},
},
},
});
});

describe('with no results', () => {
it('returns zero values', async () => {
const indicesStats = jest.fn().mockResolvedValueOnce({});

expect(await task?.executor({ indices, indicesStats } as any)).toEqual({
indices: {
shards: {
total: 0,
},
all: {
total: {
docs: {
count: 0,
},
store: {
size_in_bytes: 0,
},
},
},
},
});
});
});
});
});
Loading