-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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] Automatically create APM index pattern #34338
[APM] Automatically create APM index pattern #34338
Conversation
…on server startup if one isn't found.
💔 Build Failed |
*/ | ||
import { Server } from 'hapi'; | ||
// @ts-ignore | ||
import { getSavedObjects } from '../../../../../../src/legacy/core_plugins/kibana/server/tutorials/apm/saved_objects/get_saved_objects'; |
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 need to find a better way to share this code between the plugin and core, because this fails production build.
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.
perhaps it belongs in a @kbn
linked package at packages/kbn-plugin-apm
or something?
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 think you'll need to talk to the platform team about this. This is a good example of a shared dependency that we want to import without too much hassle.
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.
This should be fine after we merge: #32722
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.
relative imports across x-pack and core should be possible soon once #32722 merges, so i'll hold off until then.
@@ -0,0 +1,16 @@ | |||
/* |
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 should probably move this file to apm/server/lib/helpers/
search: `"${apmIndexPatternTitle}"`, | ||
searchFields: ['title'], | ||
perPage: 200 | ||
}); |
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 seem to recall that the saved objects API uses some fuzzy match and not exact match. So the search query might return results, even if the exact APM index doesn't exist.
Right now it looks like you expect it to match exactly (apmIndexPatternSearchResp.total === 0
) - please double check this.
if we expect it to be an exact match, why perPage: 200
? Shouldn't it then be perPage: 1
?
}); | ||
if (apmIndexPatternSearchResp.total === 0) { | ||
const savedObjects = getSavedObjects(apmIndexPatternTitle); | ||
await savedObjectsClient.bulkCreate(savedObjects, { overwrite: false }); |
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'm wondering if we should just ditch the logic above, and create the index pattern every time - if we keep overwrite: false
it should be the same, right?
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.
Btw. i can see that POSTing a big JSON file is heavier than doing a light GET. But since we only do it on startup I don't think it's that bad.
And due to the fuzziness issues I mentioned with the search, it might be the more solid approach.
Overall this looks really good 👍 |
💔 Build Failed |
Closes #33119 by automatically creating the apm index pattern on server startup if one isn't found.