Skip to content

K8s nav dynamic & install integrations based on data#16

Closed
jennypavlova wants to merge 3 commits into
k8s-navigation-dashboardsfrom
k8s-nav-dynamic
Closed

K8s nav dynamic & install integrations based on data#16
jennypavlova wants to merge 3 commits into
k8s-navigation-dashboardsfrom
k8s-nav-dynamic

Conversation

@jennypavlova
Copy link
Copy Markdown
Owner

@jennypavlova jennypavlova commented Jun 23, 2025

Summary

The PR proves the dynamic functionality of the side nav based on different user k8s metrics data.

Case 1 Only OTel / Semconv data in the env

only_otel_720p.mov

Case 2 Only Metricbeat / ECS data

only_ecs.mov

Case 3 Mix of OTel / Semconv & Metricbeat / ECS data

ecs_and_otel_720p.mov

Case 4 Add data case:

image

@jennypavlova jennypavlova self-assigned this Jun 23, 2025
@jennypavlova jennypavlova marked this pull request as ready for review June 25, 2025 17:33
Copy link
Copy Markdown

@rmyz rmyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +37 to +53
const metricbeatData = await core.elasticsearch.client.asCurrentUser.search({
index: 'metrics-kubernetes*',
ignore_unavailable: true,
allow_no_indices: true,
track_total_hits: true,
terminate_after: 1,
size: 0,
query: {
bool: {
should: [
{ term: { ['event.module']: KUBERNETES } },
{ term: { ['agent.type']: METRICBEAT } },
],
minimum_should_match: 1,
},
},
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed with Roshan, should we focus on otel and leave this and the toggle out?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove them in the next PR and focus on OTel only (so all metricbeat logic here will be removed)

size: 0,
query: {
bool: {
filter: [{ term: { ['data_stream.dataset']: '*.otel' } }],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
filter: [{ term: { ['data_stream.dataset']: '*.otel' } }],
filter: [{ term: { ['data_stream.dataset']: 'k8sclusterreceiver.otel' } }],

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found other indices that can also indicate we have k8s data, but this is too broad, I agree. I am wondering if the k8sclusterreceiver.otel will always have data in case of Kubernetes data? It should be the case right?

},
});

const otelData = await core.elasticsearch.client.asCurrentUser.search({
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit on the fence about checking the existence of data due to possible performance issues. I think it's interesting for the PoC, though, to understand how to provide a good UX

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a better indication if we have otel data or not - I would keep it for now for the PoC purpose but it would be nice to have the semconv entity definition present if the otel data is present or the integration is installed 🤔 probably that's not the right place and we should rethink this and make it scalable

Comment on lines +85 to +101
const packageInstalled = installedPackage ? [installedPackage] : [];

// Maybe separate the ecs / otel cases in the future
if ((hasEcsData ?? hasOtelData) && !installedPackage) {
// System package is always required
await packageClient?.ensureInstalledPackage({ pkgName: 'system' });
// Kubernetes package is required for both classic kubernetes and otel
await packageClient?.ensureInstalledPackage({ pkgName: 'kubernetes' });
const installedKubernetes = await packageClient?.getInstallation(KUBERNETES);
if (installedKubernetes) packageInstalled.push(installedKubernetes);
// Kubernetes otel package is required only for otel
if (hasOtelData && !hasEcsData) {
await packageClient?.ensureInstalledPackage({ pkgName: 'kubernetes_otel' });
const installedOtelKubernetes = await packageClient?.getInstallation('kubernetes_otel');
if (installedOtelKubernetes) packageInstalled.push(installedOtelKubernetes);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same from the onboarding flow?

@jennypavlova
Copy link
Copy Markdown
Owner Author

This PR will be kept as a reference, and I will open a new one focusing on OTel and simplifying the logic (as this still proves what we can do using the integrations and semconv entity config. We won't proceed with this one, but thanks for reviewing tho @rmyz and @crespocarlos :)

jennypavlova pushed a commit that referenced this pull request Apr 29, 2026
## Summary

Closes elastic#254714 

- Adds Claude reviewer using [GH Agentic
Workflows](https://github.github.com/gh-aw/introduction/how-they-work/)
- Adds workflow handling of @/claude mentions in comments
- Adds a `code-reviewer` subagent for reuse in workflows.
- Proxies through LiteLLM for Opus 4.7 model

### Testing

https://github.com/elastic/kibana/actions/workflows/reviewer-claude.lock.yml

This branch is directly in `elastic/kibana` rather than my fork and has
been running the workflow for reviews during development with
`pull_request` as the event, rather than the final state of
`pull_request_target` events. You can see comments Claude has posted and
responses to being mentioned. "No issues found" comments have been moved
to a `no-op` instead, the status checks reflect when the agent runs
anyways.

Will need to follow up with a proper `pull_request_target` run after
merge. There are some security settings automatically applied during
compilation that we cannot verify yet. Since we're gated behind the
label, there shouldn't be any issues immediately post merge with the
workflow running unnecessarily.

### Note for Reviewers
- It isn't necessary to review the two `lock` files. They are compiled
and generated files, but must be committed for the workflow to run.
- Open to different model versions, don't have a strong preference
towards Opus 4.7

## Cache and cost analysis

Tested ten Opus 4.7 (`llm-gateway/claude-opus-4-7`) reviewer runs with
`ENABLE_PROMPT_CACHING_1H` enabled. The cache is active, but the data
includes dirty cases: prompt changes, long gaps outside the useful cache
window, regular PR review runs, and comment-reply runs.

| Run | Event | Head | Cost | Turns | Cache read | Cache write | Output
| Effective tokens |
| --- | --- | --- | ---: | ---: | ---: | ---: | ---: | ---: |
| [#15](https://github.com/elastic/kibana/actions/runs/25011054153) |
`pull_request` | `2076d4f` | $1.04 | 14 | 809,672 | 80,897 | 5,169 |
182,559 |
| [#16](https://github.com/elastic/kibana/actions/runs/25011557405) |
`pull_request` | `382357b` | $1.32 | 21 | 1,495,919 | 70,406 | 5,226 |
240,928 |
| [#17](https://github.com/elastic/kibana/actions/runs/25012064350) |
`pull_request` | `e93fe0b` | $1.33 | 23 | 1,378,836 | 50,413 | 13,022 |
240,413 |
| [#18](https://github.com/elastic/kibana/actions/runs/25013588252) |
`pull_request` | `04ded1d` | $1.20 | 24 | 1,453,092 | 39,648 | 8,876 |
220,490 |
| [#19](https://github.com/elastic/kibana/actions/runs/25030187214) |
`pull_request` | `88a2c7e` | $1.41 | 16 | 1,165,558 | 99,949 | 8,171 |
249,210 |
| [#21](https://github.com/elastic/kibana/actions/runs/25030484848) |
`pr_review_comment` | `88a2c7e` | $0.45 | 9 | 512,383 | 19,390 | 3,035 |
82,782 |
| [#23](https://github.com/elastic/kibana/actions/runs/25031202052) |
`pull_request` | `66b198f` | $1.39 | 18 | 1,230,107 | 78,104 | 11,420 |
246,818 |
| [elastic#26](https://github.com/elastic/kibana/actions/runs/25033088024) |
`pull_request` | `d59d8c0` | $1.50 | 22 | 1,428,302 | 90,868 | 8,885 |
269,265 |
| [elastic#27](https://github.com/elastic/kibana/actions/runs/25034181833) |
`pull_request` | `d1052ef` | $1.73 | 24 | 2,016,691 | 81,113 | 8,737 |
317,759 |
| [elastic#28](https://github.com/elastic/kibana/actions/runs/25034226594) |
`pr_review_comment` | `d1052ef` | $1.18 | 23 | 1,453,250 | 36,528 |
7,788 | 213,038 |

| Run | First request cache read | First request cache write | Notes |
| --- | ---: | ---: | --- |
| 15 | 0 | 51,751 | Cold 1-hour cache write. |
| 16 | 35,886 | 15,905 | Stable prefix reused. |
| 17 | 35,886 | 15,907 | Same prefix hit pattern. |
| 18 | 35,886 | 15,911 | Same prefix hit pattern. |
| 19 | 0 | 53,676 | Cold again after long gap + prompt/head changes. |
| 21 | 45,471 | 8,375 | Comment-reply flow reused 19 prefix and wrote a
smaller suffix. |
| 23 | 37,245 | 16,587 | New commit/prompt still reused part of the
1-hour prefix. |
| 26 | 0 | 53,761 | Cold again after another gap + prompt/head changes.
|
| 27 | 37,245 | 16,511 | Warm PR run on a new head/prompt hash. |
| 28 | 45,465 | 8,461 | Comment-reply flow reused 27 prefix, but still
ran long. |

Summary:
- All observed cache creation was 1-hour (`ephemeral_1h_input_tokens`);
no 5-minute writes were observed.
- Average observed cost is about $1.26/run across all ten runs, or about
$1.37 for normal `pull_request` runs excluding comment replies.
- Comment replies are not inherently cheap: run 21 was $0.45 because it
was narrow and 9 turns, while run 28 was $1.18 because it took 23 turns.
- Cold starts appear after long gaps or prompt churn (runs 15, 19, 26);
warm runs still reuse partial prefixes even when the commit/prompt hash
changes.
- Next tuning is comparing 5-minute vs 1-hour caching, testing cheaper
models against review quality, and auditing trace data for unnecessary
output or artifact reads.

---------

Co-authored-by: Tyler Smalley <tyler.smalley@elastic.co>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants