Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[SLO] Alerts embeddable #169910
[SLO] Alerts embeddable #169910
Changes from all commits
5da91f9
81c9252
f183702
aa15c70
4f6dbe7
056ab91
65fe769
8c3c95c
7fb4777
c723f2d
94ab9ef
5a59d64
854f76f
9d0487d
87faa7f
b3ad651
f27a8f6
62d906a
2b69284
70b0f21
ecc91f9
83f1f90
385bd08
68d66ad
dfc055d
7d32548
aea5896
d047623
80995f2
8d19534
2ee36aa
e5e5c0c
9f97e96
899ff79
2320c54
70bfa4d
cb49f89
2623b68
90d96d4
5328bad
97996e5
9606eb5
feabba9
1a29174
b7cc03a
cf20b6c
423238d
80dd64f
7f71d87
72d3be6
931da96
56e269e
beeb82f
f16c003
7688768
0145bab
a36e199
59d3d55
5d64d4c
c56a012
62d1de9
a536f46
f9158d0
62d4242
6e2366b
83caa3c
4206117
32bdc03
daa44d1
a1acde7
26fec24
aef25b5
01c11ab
a8b6a1a
a1a4e26
2c6a4af
f31d0d0
9c025b2
f4a196c
74a9797
30a554f
400404a
2ff0c2e
aef588d
644fa72
6e0bb57
184defc
8616182
1c4f446
fc74b7e
072d6ca
3432e02
4cabd19
efbd59e
4a3e44f
5919347
e0fca14
137ff37
80dc85d
7aba99d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
you should add a try/catch here since
JSON.stringify
can throw an errorThere 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 -> normally we do that
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.
Is this a copy of https://github.com/elastic/kibana/blob/main/x-pack/plugins/observability/public/components/alerts_table/render_cell_value.tsx? If yes, why do we need to copy it?
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.
@maryam-saeidi Yes you are right it is a copy, with a few differences. I wanted to customize the columns of the SLO alerts table and I created a separate alert table configuration for SLO embeddable. Here are the differences with current alert table configuration
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.
@maryam-saeidi @XavierM I'll create a separate issue to address nit feedback from Xavier. @maryam-saeidi We can discuss how we better share things between alerts table in the Dashboard app and O11y alerts page
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 it is useful to have a rule name column by default for the Alerts page as well. I created a draft PR to discuss it with Vinay. (It was related to this feedback)
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.
Yep I saw this one. Makes sense to have a rule column in both cases!