-
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
[Logs Overview] Overview component (iteration 1) #191899
[Logs Overview] Overview component (iteration 1) #191899
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
8e940d2
to
0b7fc2c
Compare
@patpscal @LucaWintergerst @alex-fedotyev There are a few question that arose while implementing this. I would appreciate your input on them: Location of the "open in discover" linkThe designs show the "open in discover" link to the right of the time picker. In most places that this component would be used, though, the page already has a time picker as part of the page layout. Am I correct assuming that we don't want to duplicate that time picker as it would be confusing to the user? If so, the layout of that part of the page is not under our control and therefore inserting the "open in discover" button in that location would be tricky. Do we want to look for an alternative location that would work with those pages too? Colors of the "change type" badgesWe need to map the technical change types to the change types shown in the UI. From looking at the design the mapping is not obvious to me, so I would appreciate your input. These are the technical types that we're currently handling in the code:
Sorting by "change type"What sorting criteria do we want to use to sort the "Change type" column by? Just sorting the text of the types mentioned above alphabetically is probably not the most useful. We probably want a custom sort order that reflects what we consider to be "important". Have you put any thought into that or should I come up with something? |
@weltenwort - I am still confused with the description of the change types, trying to map those to the simple terms. |
I'll leave the open in discover button decision to someone else. Colors Sorting |
@LucaWintergerst - default sorting is going to be by "Change at" datetime field which will allow to show the changes above the systemic patterns, and also make it chronological. I wonder if we should do the opposite and never expose the p-value in any shape, and also make the "Change type" to be just the inline badge and not the column. |
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
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.
packages/kbn-management/settings/setting_ids/index.ts lgtm
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.
LGTM.
if (environment === ENVIRONMENT_ALL.value) { | ||
return [serviceNameFilter]; | ||
} else { | ||
return [ | ||
{ | ||
bool: { | ||
filter: [ | ||
serviceNameFilter, | ||
{ | ||
term: { | ||
[SERVICE_ENVIRONMENT]: environment, | ||
}, | ||
}, | ||
], | ||
}, | ||
}, | ||
{ | ||
bool: { | ||
filter: [serviceNameFilter], | ||
must_not: [ | ||
{ | ||
exists: { | ||
field: SERVICE_ENVIRONMENT, | ||
}, | ||
}, | ||
], | ||
}, | ||
}, | ||
]; | ||
} | ||
} |
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 simplify this by using environmentQuery?
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.
hm. not sure. I modeled this after the KQL query used for the old logs stream, which includes both the logs with the given environment and those not having an environment at all. The environmentQuery
function doesn't do that. Can you help me understand whether a change in the query semantics would be desirable here?
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.
Telemetry changes LGTM
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.
Amazing work 👏
(Approving based on the iterative improvements that will come and tests are TODO currently).
…dable-iteration-1
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
Total ESLint disabled count
History
cc @weltenwort |
Starting backport for target branches: 8.x |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
This introduces a "Logs Overview" component for use in solution UIs behind a feature flag. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 15bccdf) # Conflicts: # .github/CODEOWNERS # src/plugins/telemetry/schema/oss_plugins.json
This was reverted with a31b16e because |
Good to know, thank you. Sounds like that label should be the default, then? |
This is a re-submission of #191899, which was reverted due to a storybook build problem. This introduces a "Logs Overview" component for use in solution UIs behind a feature flag. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
…#195673) This is a re-submission of elastic#191899, which was reverted due to a storybook build problem. This introduces a "Logs Overview" component for use in solution UIs behind a feature flag. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Kerry Gallagher <[email protected]> Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 0caea22) # Conflicts: # .github/CODEOWNERS # src/plugins/telemetry/schema/oss_plugins.json
…195673) (#195742) # Backport This will backport the following commits from `main` to `8.x`: - [[Logs Overview] Overview component (iteration 1) (attempt 2) (#195673)](#195673) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Felix Stürmer","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-10T10:46:25Z","message":"[Logs Overview] Overview component (iteration 1) (attempt 2) (#195673)\n\nThis is a re-submission of #191899, which was reverted due to\r\na storybook build problem. This introduces a \"Logs Overview\" component for use in solution UIs\r\nbehind a feature flag.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Kerry Gallagher <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"0caea22006591486fbfd80d7899e116743acd8a2","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Logs UI","v9.0.0","release_note:feature","backport:prev-minor","ci:build-storybooks","ci:project-deploy-observability","Team:obs-ux-logs","Team:obs-ux-infra_services"],"number":195673,"url":"https://github.com/elastic/kibana/pull/195673","mergeCommit":{"message":"[Logs Overview] Overview component (iteration 1) (attempt 2) (#195673)\n\nThis is a re-submission of #191899, which was reverted due to\r\na storybook build problem. This introduces a \"Logs Overview\" component for use in solution UIs\r\nbehind a feature flag.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Kerry Gallagher <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"0caea22006591486fbfd80d7899e116743acd8a2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195673","number":195673,"mergeCommit":{"message":"[Logs Overview] Overview component (iteration 1) (attempt 2) (#195673)\n\nThis is a re-submission of #191899, which was reverted due to\r\na storybook build problem. This introduces a \"Logs Overview\" component for use in solution UIs\r\nbehind a feature flag.\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <[email protected]>\r\nCo-authored-by: Kerry Gallagher <[email protected]>\r\nCo-authored-by: Elastic Machine <[email protected]>","sha":"0caea22006591486fbfd80d7899e116743acd8a2"}}]}] BACKPORT-->
📝 Summary
This introduces a "Logs Overview" component for use in solution UIs behind a feature flag.
change_point
agg fails withIllegalArgumentException
while indexing an array elasticsearch#112805🏴☠️ Feature flag
The new logs overview is currently visible in the "Logs" tab of APM services when the feature flag
observability:newLogsOverview
is enabled in the advanced settings:🎨 Previews
In APM UI
In the hosts UI
No log entries
change_point
Elasticsearch aggregation extensively it is affected bychange_point
agg fails withIllegalArgumentException
while indexing an array elasticsearch#112805.❓ Open questions
See: #191899 (comment)
change_point
agg have some limitations:step
, aspike
and adistribution_change
when just a few buckets are different. So it might be advisable to use the change point buckets detected and re-interpret their meaning using the histogram.🕵️♀️ Review notes
xstate5
. Similarly,@xstate5/react
is available.Release Note
Add experimental logs overview to the observability hosts and service overviews