-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Inject agent config directly into APM Fleet policies #99193
[APM] Inject agent config directly into APM Fleet policies #99193
Conversation
Pinging @elastic/apm-ui (Team:apm) |
💔 Build Failed
Failed CI StepsMetrics [docs]Public APIs missing comments
Async chunks
Page load bundle
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work. Just added some small comments.
x-pack/plugins/apm/public/components/app/Settings/AgentConfigurations/index.tsx
Show resolved
Hide resolved
const isFleetSyncLoading = | ||
packagePolicyInputStatus !== FETCH_STATUS.FAILURE && | ||
packagePolicyInputStatus !== FETCH_STATUS.SUCCESS; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you use Loading instead?
const isFleetSyncLoading = | |
packagePolicyInputStatus !== FETCH_STATUS.FAILURE && | |
packagePolicyInputStatus !== FETCH_STATUS.SUCCESS; | |
const isFleetSyncLoading = packagePolicyInputStatus === FETCH_STATUS.LOADING |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also need it to be true when the status is NOT_INITIATED
,i'll probably just include that in the check along with your suggestion.
const { ['@timestamp']: lastTimestamp } = response.hits.hits[0] | ||
._source as { '@timestamp': number }; | ||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it necessary?
}, | ||
}); | ||
const agentConfigurations = configurationsSearchResponse.hits.hits | ||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use Type guards to avoid this @ts-ignore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll see what i can do, I did it because _source
has unknown
property types when querying with the alertingEsClient
@@ -262,6 +284,57 @@ const agentConfigurationAgentNameRoute = createApmServerRoute({ | |||
}, | |||
}); | |||
|
|||
// enables syncing with fleet-managed apm package policy | |||
const agentConfigurationFleetSync = createApmServerRoute({ | |||
endpoint: 'POST /api/apm/settings/agent-configuration/fleet_sync', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also add an API to stop the sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want the button to also be a toggle, we can do that, otherwise the user is able to stop syncing by navigating to Stack management and delete the rule.
@@ -62,13 +99,38 @@ export function AgentConfigurations() { | |||
<EuiTitle size="s"> | |||
<h2> | |||
{i18n.translate( | |||
'xpack.apm.agentConfig.configurationsPanelTitle', | |||
'xpack.apm.agentConfig.configurationsPanel.title', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was there already was correct according to the Kibana i18n guidelines.
savedObjectsClient, | ||
{ kuery: 'ingest-package-policies.package.name:apm' } | ||
); | ||
const { config } = packagePolicies.items[0].inputs[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If items
is an empty array it returns undefined when you request the first element.
Addresses #95501.