-
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] Display automatic deployment annotations correctly #102020
Conversation
Pinging @elastic/apm-ui (Team:apm) |
}); | ||
} | ||
|
||
await es.indices.create({ |
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.
Do you really need to create the index before calling the API? I thought that happened automatically
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 you create an index template yes, otherwise you have to create it manually right?
}); | ||
|
||
const docs = dates.flatMap((date, index) => { | ||
const defaults = { |
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.
nit: naming
const defaults = { | |
const defaultAnnotation = { |
or
const defaults = { | |
const baseAnnotation = { |
|
||
await es.bulk({ | ||
index: indexName, | ||
body: docs.flatMap((doc) => [{ index: {} }, doc]), |
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 are you adding an empty doc for every annotation doc?
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 not, I think? This is how the bulk API works: an operation (in this case index
), followed by the document.
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.
Ah yes. i forgot that's how it works. So weird that the ES client doesn't abstract that away if you ask me.
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.
Thanks for adding a test
jenkins run the e2e |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
@dgieselaar is there an easy way to test it? |
Closes #101760.