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.

Sign up for GitHub

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] Added Android Agent name and icon #136598

Merged
merged 10 commits into from
Jul 19, 2022
Merged

[APM] Added Android Agent name and icon #136598

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
96fbc40
Added Android Agent icon
AlexanderWert Jul 19, 2022
6bf4975
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 19, 2022
5d50044
Added agent name to telemetry
AlexanderWert Jul 19, 2022
d33234d
Replaced usage of isIOSAgentName to isMobileAgentName. Added tests f…
AlexanderWert Jul 19, 2022
ca5bbcb
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 19, 2022
5176111
Update telemetry collection for android/java agent
AlexanderWert Jul 19, 2022
141e23f
Make opentelemetry/swift not being recognized as mobile agent
AlexanderWert Jul 19, 2022
bdcf7fd
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 19, 2022
f44fcf4
Fix Jest tests
AlexanderWert Jul 19, 2022
a24c429
Update apm_telemetry.test.ts.snap
AlexanderWert Jul 19, 2022
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
64 changes: 62 additions & 2 deletions x-pack/plugins/apm/common/agent_name.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
isJavaAgentName,
isRumAgentName,
isIosAgentName,
isAndroidAgentName,
isMobileAgentName,
isServerlessAgent,
} from './agent_name';

Expand Down Expand Up @@ -60,13 +62,13 @@ describe('agent name helpers', () => {
});

describe('isIosAgentName', () => {
describe('when the agent name is js-base', () => {
describe('when the agent name is iOS/swift', () => {
it('returns true', () => {
expect(isIosAgentName('iOS/swift')).toEqual(true);
});
});

describe('when the agent name is rum-js', () => {
describe('when the agent name is ios/swift', () => {
it('returns true', () => {
expect(isIosAgentName('ios/swift')).toEqual(true);
});
Expand All @@ -85,6 +87,64 @@ describe('agent name helpers', () => {
});
});

describe('isAndroidAgentName', () => {
describe('when the agent name is android/java', () => {
it('returns true', () => {
expect(isAndroidAgentName('android/java')).toEqual(true);
});
});

describe('when the agent name is opentelemetry/java', () => {
it('returns false', () => {
expect(isAndroidAgentName('opentelemetry/java')).toEqual(false);
});
});

describe('when the agent name is something else', () => {
it('returns false', () => {
expect(isAndroidAgentName('not android')).toEqual(false);
});
});
});

describe('isMobileAgentName', () => {
describe('when the agent name is android/java', () => {
it('returns true', () => {
expect(isMobileAgentName('android/java')).toEqual(true);
});
});

describe('when the agent name is iOS/swift', () => {
it('returns true', () => {
expect(isMobileAgentName('iOS/swift')).toEqual(true);
});
});

describe('when the agent name is ios/swift', () => {
it('returns true', () => {
expect(isMobileAgentName('ios/swift')).toEqual(true);
});
});

describe('when the agent name is opentelemetry/swift', () => {
it('returns true', () => {
expect(isMobileAgentName('opentelemetry/swift')).toEqual(true);
});
});

describe('when the agent name is opentelemetry/java', () => {
it('returns false', () => {
expect(isMobileAgentName('opentelemetry/java')).toEqual(false);
});
});

describe('when the agent name is something else', () => {
it('returns false', () => {
expect(isMobileAgentName('not mobile')).toEqual(false);
});
});
});

describe('isServerlessAgent', () => {
describe('when the runtime name is AWS_LAMBDA', () => {
it('returns true', () => {
Expand Down
19 changes: 19 additions & 0 deletions x-pack/plugins/apm/common/agent_name.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const AGENT_NAMES: AgentName[] = [
'python',
'ruby',
'rum-js',
'android/java',
...OPEN_TELEMETRY_AGENT_NAMES,
];

Expand All @@ -64,6 +65,15 @@ export function isRumAgentName(
return RUM_AGENT_NAMES.includes(agentName! as AgentName);
}

export function isMobileAgentName(
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be used

Copy link
Member Author

@AlexanderWert AlexanderWert Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @gbamparop !

I pushed a commit now in which I replaced the usage of isOSAgentName with isMobileAgentName in all the places in the APM UI. Since the Android agent should be treated in the same way as the iOS agent.

Copy link
Member Author

@AlexanderWert AlexanderWert Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What is not clear to me is:

  • is the method normalizeAgentName used anywhere? I couldn't find anything
  • why did we define opentelemetry/swift to be an iOS agent / mobile agent. We might need to change this as well. Because if a MacOS client application is instrumented with the OpenTelemetry SDK, this would be recognized and treated as a mobile agent. I don't think this is what we want. Do have any concerns with changing this as well (so that only iOS/swift or ios/swift would be recognized as iOSAgent / mobile agent)?
  • Why do we hide the dependencies view for the iOSAgent / mobile agent?

Copy link
Contributor

@gbamparop gbamparop Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for updating this!

  • is the method normalizeAgentName used anywhere? I couldn't find anything

It seems to have been introduced by #102346 but it's not used

  • why did we define opentelemetry/swift

It makes sense, maybe we hadn't considered that at the time (see #103485 (comment))

It was raised as an issue #108510

Copy link
Member Author

@AlexanderWert AlexanderWert Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @gbamparop

  • is the method normalizeAgentName used anywhere? I couldn't find anything

It seems to have been introduced by #102346 but it's not used

OK, I deleted it and simplified the code there.

  • why did we define opentelemetry/swift

It makes sense, maybe we hadn't considered that at the time

👍 I reverted it to be just iOS/swift / ios/swift

It was raised as an issue #108510

I'll keep it as is for now and continue the discussion on this aspect in a separate thread.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, do you think there's another field that we could use to find if an app instrumented with opentelemetry/swift is on iOS?

agentName?: string
): agentName is 'iOS/swift' | 'android/java' {
return (
isIosAgentName(agentName! as AgentName) ||
isAndroidAgentName(agentName! as AgentName)
);
}

export function normalizeAgentName<T extends string | undefined>(
agentName: T
): T | string {
Expand All @@ -79,6 +89,10 @@ export function normalizeAgentName<T extends string | undefined>(
return 'ios';
}

if (isAndroidAgentName(agentName)) {
return 'android';
}

return agentName;
}

Expand All @@ -94,3 +108,8 @@ export function isJRubyAgent(agentName?: string, runtimeName?: string) {
export function isServerlessAgent(runtimeName?: string) {
return runtimeName?.toLowerCase().startsWith('aws_lambda');
}

export function isAndroidAgentName(agentName?: string) {
const lowercased = agentName && agentName.toLowerCase();
return lowercased === 'android/java';
}
6 changes: 3 additions & 3 deletions x-pack/plugins/apm/public/components/app/service_overview/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import React from 'react';
import { useHistory } from 'react-router-dom';
import {
isRumAgentName,
isIosAgentName,
isMobileAgentName,
isServerlessAgent,
} from '../../../../common/agent_name';
import { AnnotationsContextProvider } from '../../../context/annotations/annotations_context';
Expand Down Expand Up @@ -78,7 +78,7 @@ export function ServiceOverview() {
: chartHeight;
const rowDirection = isSingleColumn ? 'column' : 'row';
const isRumAgent = isRumAgentName(agentName);
const isIosAgent = isIosAgentName(agentName);
const isMobileAgent = isMobileAgentName(agentName);
const isServerless = isServerlessAgent(runtimeName);
const router = useApmRouter();
const dependenciesLink = router.link('/services/{serviceName}/dependencies', {
Expand Down Expand Up @@ -199,7 +199,7 @@ export function ServiceOverview() {
)}
</EuiFlexGroup>
</EuiFlexItem>
{!isRumAgent && !isIosAgent && !isServerless && (
{!isRumAgent && !isMobileAgent && !isServerless && (
<EuiFlexItem>
<EuiFlexGroup
direction="column"
Expand Down
4 changes: 2 additions & 2 deletions ...gins/apm/public/components/routing/templates/apm_service_template/analyze_data_button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useKibana } from '@kbn/kibana-react-plugin/public';
import { createExploratoryViewUrl } from '@kbn/observability-plugin/public';
import { ALL_VALUES_SELECTED } from '@kbn/observability-plugin/public';
import {
isIosAgentName,
isMobileAgentName,
isRumAgentName,
} from '../../../../../common/agent_name';
import {
Expand Down Expand Up @@ -49,7 +49,7 @@ export function AnalyzeDataButton() {
const canShowDashboard = services.application?.capabilities.dashboard.show;

if (
(isRumAgentName(agentName) || isIosAgentName(agentName)) &&
(isRumAgentName(agentName) || isMobileAgentName(agentName)) &&
rangeFrom &&
canShowDashboard &&
rangeTo
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/apm/public/components/routing/templates/apm_service_template/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { omit } from 'lodash';
import React from 'react';
import { enableInfrastructureView } from '@kbn/observability-plugin/public';
import {
isIosAgentName,
isMobileAgentName,
isJavaAgentName,
isJRubyAgent,
isRumAgentName,
Expand Down Expand Up @@ -145,7 +145,7 @@ export function isMetricsTabHidden({
!agentName ||
isRumAgentName(agentName) ||
isJavaAgentName(agentName) ||
isIosAgentName(agentName) ||
isMobileAgentName(agentName) ||
isJRubyAgent(agentName, runtimeName) ||
isServerlessAgent(runtimeName)
);
Expand Down Expand Up @@ -221,7 +221,7 @@ function useTabs({ selectedTab }: { selectedTab: Tab['key'] }) {
defaultMessage: 'Dependencies',
}),
hidden:
!agentName || isRumAgentName(agentName) || isIosAgentName(agentName),
!agentName || isRumAgentName(agentName) || isMobileAgentName(agentName),
},
{
key: 'errors',
Expand Down Expand Up @@ -286,7 +286,7 @@ function useTabs({ selectedTab }: { selectedTab: Tab['key'] }) {
defaultMessage: 'Logs',
}),
hidden:
!agentName || isRumAgentName(agentName) || isIosAgentName(agentName),
!agentName || isRumAgentName(agentName) || isMobileAgentName(agentName),
},
{
key: 'profiling',
Expand Down
7 changes: 7 additions & 0 deletions x-pack/plugins/apm/public/components/shared/agent_icon/get_agent_icon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
isIosAgentName,
isRumAgentName,
isJavaAgentName,
isAndroidAgentName,
OPEN_TELEMETRY_AGENT_NAMES,
} from '../../../../common/agent_name';
import { AgentName } from '../../../../typings/es_schemas/ui/fields/agent';
Expand All @@ -31,6 +32,7 @@ import darkPhpIcon from './icons/php_dark.svg';
import darkRumJsIcon from './icons/rumjs_dark.svg';
import rustIcon from './icons/rust.svg';
import darkRustIcon from './icons/rust_dark.svg';
import androidIcon from './icons/android.svg';

const agentIcons: { [key: string]: string } = {
dotnet: dotNetIcon,
Expand All @@ -47,6 +49,7 @@ const agentIcons: { [key: string]: string } = {
ruby: rubyIcon,
rum: rumJsIcon,
rust: rustIcon,
android: androidIcon,
};

const darkAgentIcons: { [key: string]: string } = {
Expand Down Expand Up @@ -77,6 +80,10 @@ export function getAgentIconKey(agentName: string) {
return 'ios';
}

if (isAndroidAgentName(lowercasedAgentName)) {
return 'android';
}

// Remove "opentelemetry/" prefix
const agentNameWithoutPrefix = lowercasedAgentName.replace(
/^opentelemetry\//,
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/apm/public/components/shared/agent_icon/icons/android.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions x-pack/plugins/apm/server/lib/apm_telemetry/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const apmPerAgentSchema: Pick<
// TODO: Find a way for `@kbn/telemetry-tools` to understand and evaluate expressions.
// In the meanwhile, we'll have to maintain these lists up to date (TS will remind us to update)
services_per_agent: {
'android/java': long,
dotnet: long,
'iOS/swift': long,
go: long,
Expand All @@ -98,6 +99,7 @@ const apmPerAgentSchema: Pick<
'opentelemetry/webjs': long,
},
agents: {
'android/java': agentSchema,
dotnet: agentSchema,
'iOS/swift': agentSchema,
go: agentSchema,
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/apm/typings/es_schemas/ui/fields/agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export type ElasticAgentName =
| 'python'
| 'dotnet'
| 'ruby'
| 'php';
| 'php'
| 'android/java';

export type OpenTelemetryAgentName =
| 'otlp'
Expand Down
87 changes: 87 additions & 0 deletions x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2602,6 +2602,9 @@
"properties": {
"services_per_agent": {
"properties": {
"android/java": {
"type": "long"
},
"dotnet": {
"type": "long"
},
Expand Down Expand Up @@ -2672,6 +2675,90 @@
},
"agents": {
"properties": {
"android/java": {
"properties": {
"agent": {
"properties": {
"version": {
"type": "array",
"items": {
"type": "keyword"
}
}
}
},
"service": {
"properties": {
"framework": {
"properties": {
"name": {
"type": "array",
"items": {
"type": "keyword"
}
},
"version": {
"type": "array",
"items": {
"type": "keyword"
}
},
"composite": {
"type": "array",
"items": {
"type": "keyword"
}
}
}
},
"language": {
"properties": {
"name": {
"type": "array",
"items": {
"type": "keyword"
}
},
"version": {
"type": "array",
"items": {
"type": "keyword"
}
},
"composite": {
"type": "array",
"items": {
"type": "keyword"
}
}
}
},
"runtime": {
"properties": {
"name": {
"type": "array",
"items": {
"type": "keyword"
}
},
"version": {
"type": "array",
"items": {
"type": "keyword"
}
},
"composite": {
"type": "array",
"items": {
"type": "keyword"
}
}
}
}
}
}
}
},
"dotnet": {
"properties": {
"agent": {
Expand Down