[main] NETOBSERV-2608: TLS, metrics and charts#1530
Merged
jotak merged 4 commits intoMay 18, 2026
Conversation
- Add 4 new overview panels related to TLS: - TLS global usage (traffic with/without TLS) - breakdown by version - breakdown by group - breakdown by cipher suite - Add new column filter "tls" in manage columns panel - Fix issues in logQL when querying on arrays: handle looking for field presence/absence - Refactoring: introduce StructuredFlowQuery, an intermediate data structure used before the flow query is entirely serialized for HTTP. It allows to keep and manipulate structured filters for complex queries.
- Simplify TLS usage donut to show just TLS vs Other (it's
easier to achieve with prom metrics)
- Show % of TLS rather than total Pps
- Fix a couple of JS warnings
Donut legend will not be dispayed anymore when rendered on small screens. It's still available as a tooltip though.
6 tasks
Collaborator
|
@openshift-cherrypick-robot: Ignoring requests to cherry-pick non-bug issues: NETOBSERV-2608 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @openshift-cherrypick-robot. Thanks for your PR. I'm waiting for a netobserv member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds TLS tracking and TLS-specific overview panels, migrates flow queries from string-encoded filters to a typed StructuredFlowQuery, refactors back-and-forth fetch and LogQL builder/filter logic, and wires TLS metrics and UI rendering throughout frontend and backend pieces. ChangesTLS Tracking and Structured Query Model
🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 2 linked repositories, but your current plan allows 1. Analyzed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
|
New image: quay.io/netobserv/network-observability-console-plugin:3f2900e
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=3f2900e make set-plugin-image |
Collaborator
|
@openshift-cherrypick-robot: This pull request references NETOBSERV-2608 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/utils/back-and-forth.ts (1)
56-82:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winType mismatch: mixing
StructuredFlowQuerywithFlowQuerycontract.Lines 68, 69, and 71-75 spread
initialQuery: StructuredFlowQueryand addfiltersstrings.getFlowMetricsexpectsFlowQuery. Convert first:const getMetricsBNF = ( initialQuery: StructuredFlowQuery, range: number | TimeRange, orig: Filter[], swapped: Filter[] ): Promise<FlowMetricsResult> => { - // When bnf is on, this replaces the usual getMetrics with a function with same arguments that runs 3 queries and merge their results - // in order to get the ORIGINAL + SWAPPED - OVERLAP - // OVERLAP being ORIGINAL AND SWAPPED. - // E.g: if ORIGINAL is "SrcNs=foo", SWAPPED is "DstNs=foo" and OVERLAP is "SrcNs=foo AND DstNs=foo" const matchAny = initialQuery.structuredFilters.match === 'any'; const overlapFilters = matchAny ? undefined : [...orig, ...swapped]; - const promOrig = getFlowMetrics({ ...initialQuery, filters: filtersToString(orig, matchAny) }, range); - const promSwapped = getFlowMetrics({ ...initialQuery, filters: filtersToString(swapped, matchAny) }, range); + const rawQuery = structuredToRawQuery(initialQuery); + const promOrig = getFlowMetrics({ ...rawQuery, filters: filtersToString(orig, matchAny) }, range); + const promSwapped = getFlowMetrics({ ...rawQuery, filters: filtersToString(swapped, matchAny) }, range); const promOverlap = overlapFilters ? getFlowMetrics( { - ...initialQuery, + ...rawQuery, filters: filtersToString(overlapFilters, matchAny) }, range ) : Promise.resolve(undefined); return Promise.all([promOrig, promSwapped, promOverlap]).then(([rsOrig, rsSwapped, rsOverlap]) => mergeMetricsBNF(range, rsOrig, rsSwapped, rsOverlap) ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/utils/back-and-forth.ts` around lines 56 - 82, getMetricsBNF is passing initialQuery (a StructuredFlowQuery) directly into getFlowMetrics calls while overwriting filters with a string; getFlowMetrics expects a FlowQuery—fix by converting initialQuery into a FlowQuery before spreading into the calls (e.g., build a FlowQuery object from initialQuery fields, or call a converter function) and then pass that converted FlowQuery plus the filters produced by filtersToString for orig, swapped, and overlap; update the three places where getFlowMetrics is invoked (promOrig, promSwapped, promOverlap) to use the converted FlowQuery instead of initialQuery so the types align (referencing getMetricsBNF, initialQuery, getFlowMetrics, StructuredFlowQuery, FlowQuery, filtersToString).web/src/components/tabs/netflow-overview/netflow-overview.tsx (1)
80-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the unused
filterDefinitionsprop fromNetflowOverviewProps.The prop is declared at line 87 but never referenced anywhere in the component. Remove it from the interface unless it's needed for a downstream consumer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/tabs/netflow-overview/netflow-overview.tsx` around lines 80 - 97, NetflowOverviewProps declares an unused prop filterDefinitions; remove the filterDefinitions property from the NetflowOverviewProps interface (and any associated optional typing) to eliminate the dead prop declaration — update the NetflowOverviewProps interface definition (the type that includes limit, panels, recordType, metrics, etc.) by deleting the filterDefinitions: FilterDefinition[] entry and adjust any imports or downstream consumers only if they actually reference filterDefinitions elsewhere.
🧹 Nitpick comments (1)
web/src/model/flow-query.ts (1)
47-55: 💤 Low valueRemove redundant undefined assignment.
Line 51 sets
structuredFilters: undefinedbut then line 53 deletes it. Theundefinedassignment is redundant.♻️ Simplify
export const structuredToRawQuery = (q: StructuredFlowQuery): FlowQuery => { const raw = { ...q, - filters: filtersToString(q.structuredFilters.list, q.structuredFilters.match === 'any'), - structuredFilters: undefined + filters: filtersToString(q.structuredFilters.list, q.structuredFilters.match === 'any') }; delete raw.structuredFilters; return raw; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/model/flow-query.ts` around lines 47 - 55, The function structuredToRawQuery assigns structuredFilters: undefined in the constructed raw object and then immediately deletes it, which is redundant; update structuredToRawQuery to omit structuredFilters when building raw (remove the structuredFilters: undefined property and the subsequent delete) so the function simply spreads q and sets filters via filtersToString using q.structuredFilters.list and q.structuredFilters.match, leaving structuredFilters out of the returned FlowQuery.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/model/filters/logql.go`:
- Around line 276-289: The code currently treats the empty sentinel only when
it's the sole value, which causes mixed inputs (e.g. values contains "" and "x")
to be converted into an over-broad regex; change the logic to detect any empty
sentinel in values and handle it separately: when values contains "" and other
values, remove the empty sentinel from the slice that becomes LineFilter.values
(avoid appending a lineMatch with typeRegexArrayContains for the empty), and
instead combine the missing-key condition using NotContainsKeyLineFilter(key)
for the positive case (or the appropriate negated equivalent when not==true) so
the final behavior is "key is missing OR array contains any of the remaining
values"; update the code around LineFilter, the loop that appends lineMatch
entries, and the return path that currently uses RegexMatchLineFilter and
NotContainsKeyLineFilter to implement this combined semantics.
In `@web/src/components/metrics/metrics-donut.tsx`:
- Around line 150-151: The radius and innerRadius props use showLegend but the
component determines visibility with showLegendResponsive, causing wrong sizing
when the legend is hidden responsively; update the radius and innerRadius
calculations to use showLegendResponsive (instead of showLegend) so that
radius={showLegendResponsive ? dimensions.height / 3 : undefined} and
innerRadius={showLegendResponsive ? dimensions.height / 4 : undefined}, ensuring
the donut sizing matches the actual legend visibility (refer to the
radius/innerRadius props and the showLegendResponsive variable in
metrics-donut.tsx).
In `@web/src/components/tabs/netflow-overview/netflow-overview.tsx`:
- Around line 1145-1204: The tls_usage_global branch wrongly treats pctTLS==0 as
falsy and hardcodes 'n/a', and it force-asserts total with total! even though
total may be undefined; update the pctTLS display to check for a defined numeric
value (e.g. typeof pctTLS === 'number' or pctTLS !== undefined) and pass its
formatted value or t('n/a') into MetricsDonut (internalText), and change the
MetricsGraph rendering to only pass the total when it is actually defined (avoid
total!): either render emptyGraph or the donut when total is missing, or build
the metrics array conditionally (e.g. metrics and total ? [metrics, total] :
[metrics]) so you never assert a non-null total; reference symbols: pctTLS,
MetricsDonut, MetricsGraph, props.metrics.totalFlowRate, and panelError.
- Around line 410-435: The metric legend labels are hardcoded in the Result
handlers for getFlowGenericMetrics — replace the literal strings 'Total flows'
and 'TLS flows' with translated strings using the react-i18next t(...) function
(i.e. set totalFlowRate to use name: t('Total flows') and tlsFlowRate to use
name: t('TLS flows')), ensure the component has access to t (via useTranslation)
and add t to the fetch useCallback dependency array so setMetrics/currentMetrics
updates use the localized labels correctly.
In `@web/src/utils/back-and-forth.ts`:
- Around line 49-54: getFlowsBNF is currently spreading a StructuredFlowQuery
(initialQuery) and injecting a string filters field, creating a type hybrid that
violates the FlowQuery contract expected by getFlowRecords. Fix by building a
proper FlowQuery object: extract the FlowQuery-compatible properties from
initialQuery (e.g., time range, pagination, showDuplicates, any other FlowQuery
fields) and set filters to the combined string from filtersToString, instead of
spreading initialQuery directly; then call getFlowRecords with that constructed
FlowQuery. Reference: getFlowsBNF, StructuredFlowQuery, FlowQuery,
getFlowRecords, filtersToString.
---
Outside diff comments:
In `@web/src/components/tabs/netflow-overview/netflow-overview.tsx`:
- Around line 80-97: NetflowOverviewProps declares an unused prop
filterDefinitions; remove the filterDefinitions property from the
NetflowOverviewProps interface (and any associated optional typing) to eliminate
the dead prop declaration — update the NetflowOverviewProps interface definition
(the type that includes limit, panels, recordType, metrics, etc.) by deleting
the filterDefinitions: FilterDefinition[] entry and adjust any imports or
downstream consumers only if they actually reference filterDefinitions
elsewhere.
In `@web/src/utils/back-and-forth.ts`:
- Around line 56-82: getMetricsBNF is passing initialQuery (a
StructuredFlowQuery) directly into getFlowMetrics calls while overwriting
filters with a string; getFlowMetrics expects a FlowQuery—fix by converting
initialQuery into a FlowQuery before spreading into the calls (e.g., build a
FlowQuery object from initialQuery fields, or call a converter function) and
then pass that converted FlowQuery plus the filters produced by filtersToString
for orig, swapped, and overlap; update the three places where getFlowMetrics is
invoked (promOrig, promSwapped, promOverlap) to use the converted FlowQuery
instead of initialQuery so the types align (referencing getMetricsBNF,
initialQuery, getFlowMetrics, StructuredFlowQuery, FlowQuery, filtersToString).
---
Nitpick comments:
In `@web/src/model/flow-query.ts`:
- Around line 47-55: The function structuredToRawQuery assigns
structuredFilters: undefined in the constructed raw object and then immediately
deletes it, which is redundant; update structuredToRawQuery to omit
structuredFilters when building raw (remove the structuredFilters: undefined
property and the subsequent delete) so the function simply spreads q and sets
filters via filtersToString using q.structuredFilters.list and
q.structuredFilters.match, leaving structuredFilters out of the returned
FlowQuery.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85139e97-7e7f-4368-a863-107cabcbce3e
⛔ Files ignored due to path filters (1)
web/src/globals.d.tsis excluded by!**/*.d.ts
📒 Files selected for processing (30)
config/sample-config.yamlpkg/loki/flow_query.gopkg/loki/topology_query.gopkg/model/filters/logql.gopkg/prometheus/inventory.gopkg/prometheus/inventory_test.gopkg/utils/constants/constants.goweb/locales/en/plugin__netobserv-plugin.jsonweb/src/api/loki.tsweb/src/api/routes.tsweb/src/components/drawer/netflow-traffic-drawer.tsxweb/src/components/messages/empty.tsxweb/src/components/messages/error.tsxweb/src/components/metrics/metrics-donut.tsxweb/src/components/metrics/metrics-graph.tsxweb/src/components/modals/__tests__/export-modal.spec.tsxweb/src/components/modals/columns-modal.tsxweb/src/components/modals/export-modal.tsxweb/src/components/slider/scope-slider.tsxweb/src/components/tabs/netflow-overview/__tests__/netflow-overview.spec.tsxweb/src/components/tabs/netflow-overview/netflow-overview.tsxweb/src/model/config.tsweb/src/model/filters.tsweb/src/model/flow-query.tsweb/src/model/netflow-context.tsweb/src/utils/__tests__/back-and-forth.spec.tsweb/src/utils/back-and-forth.tsweb/src/utils/metrics.tsweb/src/utils/netflow-capabilities-hook.tsweb/src/utils/overview-panels.ts
💤 Files with no reviewable changes (2)
- web/src/components/messages/error.tsx
- web/src/components/messages/empty.tsx
Comment on lines
+276
to
+289
| if len(values) == 1 && (values[0] == "" || values[0] == `""`) { | ||
| // Special case, filter for n/a; note that logQL json doesn't support arrays, so we need a different solution | ||
| if not { | ||
| // key!=n/a | ||
| return RegexMatchLineFilter(key, true, ".*"), false | ||
| } | ||
| // key=n/a | ||
| return NotContainsKeyLineFilter(key), false | ||
| } | ||
| lf := LineFilter{key: key, not: not} | ||
| for _, value := range values { | ||
| lf.values = append(lf.values, lineMatch{valueType: typeRegexArrayContains, value: value}) | ||
| } | ||
| return lf | ||
| return lf, false |
Contributor
There was a problem hiding this comment.
Handle empty sentinel even when mixed with other array values.
Line 276 only treats empty as special when it is the sole value. With mixed inputs (e.g. ["", "x"]), the empty value becomes a regex that matches any array, which over-broadens results.
Proposed fix
func ArrayLineFilter(key string, values []string, not bool) (LineFilter, bool) {
- if len(values) == 1 && (values[0] == "" || values[0] == `""`) {
+ hasEmptyMatch := false
+ nonEmpty := make([]string, 0, len(values))
+ for _, v := range values {
+ if v == "" || v == `""` {
+ hasEmptyMatch = true
+ continue
+ }
+ nonEmpty = append(nonEmpty, v)
+ }
+
+ if hasEmptyMatch && len(nonEmpty) == 0 {
// Special case, filter for n/a; note that logQL json doesn't support arrays, so we need a different solution
if not {
// key!=n/a
return RegexMatchLineFilter(key, true, ".*"), false
}
// key=n/a
return NotContainsKeyLineFilter(key), false
}
- lf := LineFilter{key: key, not: not}
- for _, value := range values {
+ lf := LineFilter{key: key, not: not}
+ for _, value := range nonEmpty {
lf.values = append(lf.values, lineMatch{valueType: typeRegexArrayContains, value: value})
}
- return lf, false
+ return lf, hasEmptyMatch
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/model/filters/logql.go` around lines 276 - 289, The code currently treats
the empty sentinel only when it's the sole value, which causes mixed inputs
(e.g. values contains "" and "x") to be converted into an over-broad regex;
change the logic to detect any empty sentinel in values and handle it
separately: when values contains "" and other values, remove the empty sentinel
from the slice that becomes LineFilter.values (avoid appending a lineMatch with
typeRegexArrayContains for the empty), and instead combine the missing-key
condition using NotContainsKeyLineFilter(key) for the positive case (or the
appropriate negated equivalent when not==true) so the final behavior is "key is
missing OR array contains any of the remaining values"; update the code around
LineFilter, the loop that appends lineMatch entries, and the return path that
currently uses RegexMatchLineFilter and NotContainsKeyLineFilter to implement
this combined semantics.
Comment on lines
150
to
151
| radius={showLegend ? dimensions.height / 3 : undefined} | ||
| innerRadius={showLegend ? dimensions.height / 4 : undefined} |
Contributor
There was a problem hiding this comment.
Inconsistent radius calculation when legend is hidden responsively.
Lines 150-151 use showLegend to calculate radius, but legend rendering uses showLegendResponsive (lines 145, 149, 161). When showLegend={true} but screen width < 550px, the legend is hidden yet the donut remains small, wasting space.
Use showLegendResponsive for consistent sizing:
- radius={showLegend ? dimensions.height / 3 : undefined}
- innerRadius={showLegend ? dimensions.height / 4 : undefined}
+ radius={showLegendResponsive ? dimensions.height / 3 : undefined}
+ innerRadius={showLegendResponsive ? dimensions.height / 4 : undefined}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| radius={showLegend ? dimensions.height / 3 : undefined} | |
| innerRadius={showLegend ? dimensions.height / 4 : undefined} | |
| radius={showLegendResponsive ? dimensions.height / 3 : undefined} | |
| innerRadius={showLegendResponsive ? dimensions.height / 4 : undefined} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/metrics/metrics-donut.tsx` around lines 150 - 151, The
radius and innerRadius props use showLegend but the component determines
visibility with showLegendResponsive, causing wrong sizing when the legend is
hidden responsively; update the radius and innerRadius calculations to use
showLegendResponsive (instead of showLegend) so that
radius={showLegendResponsive ? dimensions.height / 3 : undefined} and
innerRadius={showLegendResponsive ? dimensions.height / 4 : undefined}, ensuring
the donut sizing matches the actual legend visibility (refer to the
radius/innerRadius props and the showLegendResponsive variable in
metrics-donut.tsx).
Comment on lines
+410
to
+435
| promises.push( | ||
| Result.fromPromise(getFlowGenericMetrics(fqTotal, range)).then(res => { | ||
| const totalFlowRate = res | ||
| .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'Total flows' } : undefined)) | ||
| .mapError(err => getStructuredHTTPError(err, `Total flow rate`)); | ||
| currentMetrics = { ...currentMetrics, totalFlowRate }; | ||
| setMetrics(currentMetrics); | ||
| return res.map(r => r.stats).or(emptyStats); | ||
| }) | ||
| ); | ||
| const fqTLS: StructuredFlowQuery = { | ||
| ...baseQuery, | ||
| function: 'rate', | ||
| type: 'TlsFlows', | ||
| aggregateBy: 'app' | ||
| }; | ||
| promises.push( | ||
| Result.fromPromise(getFlowGenericMetrics(fqTLS, range)).then(res => { | ||
| const tlsFlowRate = res | ||
| .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'TLS flows' } : undefined)) | ||
| .mapError(err => getStructuredHTTPError(err, `TLS flow rate`)); | ||
| currentMetrics = { ...currentMetrics, tlsFlowRate }; | ||
| setMetrics(currentMetrics); | ||
| return res.map(r => r.stats).or(emptyStats); | ||
| }) | ||
| ); |
Contributor
There was a problem hiding this comment.
Hardcoded English strings in metric names.
'Total flows' (line 413) and 'TLS flows' (line 429) are user-facing — they surface as legend labels in the donut/graph. Wrap with t(...).
🌐 Proposed fix
- .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'Total flows' } : undefined))
+ .map(success =>
+ success.metrics.length > 0 ? { ...success.metrics[0], name: t('Total flows') } : undefined
+ )
@@
- .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'TLS flows' } : undefined))
+ .map(success =>
+ success.metrics.length > 0 ? { ...success.metrics[0], name: t('TLS flows') } : undefined
+ )Note: t must then be included in the fetch useCallback deps.
As per coding guidelines: "All user-facing strings must use react-i18next for internationalization".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| promises.push( | |
| Result.fromPromise(getFlowGenericMetrics(fqTotal, range)).then(res => { | |
| const totalFlowRate = res | |
| .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'Total flows' } : undefined)) | |
| .mapError(err => getStructuredHTTPError(err, `Total flow rate`)); | |
| currentMetrics = { ...currentMetrics, totalFlowRate }; | |
| setMetrics(currentMetrics); | |
| return res.map(r => r.stats).or(emptyStats); | |
| }) | |
| ); | |
| const fqTLS: StructuredFlowQuery = { | |
| ...baseQuery, | |
| function: 'rate', | |
| type: 'TlsFlows', | |
| aggregateBy: 'app' | |
| }; | |
| promises.push( | |
| Result.fromPromise(getFlowGenericMetrics(fqTLS, range)).then(res => { | |
| const tlsFlowRate = res | |
| .map(success => (success.metrics.length > 0 ? { ...success.metrics[0], name: 'TLS flows' } : undefined)) | |
| .mapError(err => getStructuredHTTPError(err, `TLS flow rate`)); | |
| currentMetrics = { ...currentMetrics, tlsFlowRate }; | |
| setMetrics(currentMetrics); | |
| return res.map(r => r.stats).or(emptyStats); | |
| }) | |
| ); | |
| promises.push( | |
| Result.fromPromise(getFlowGenericMetrics(fqTotal, range)).then(res => { | |
| const totalFlowRate = res | |
| .map(success => | |
| success.metrics.length > 0 ? { ...success.metrics[0], name: t('Total flows') } : undefined | |
| ) | |
| .mapError(err => getStructuredHTTPError(err, `Total flow rate`)); | |
| currentMetrics = { ...currentMetrics, totalFlowRate }; | |
| setMetrics(currentMetrics); | |
| return res.map(r => r.stats).or(emptyStats); | |
| }) | |
| ); | |
| const fqTLS: StructuredFlowQuery = { | |
| ...baseQuery, | |
| function: 'rate', | |
| type: 'TlsFlows', | |
| aggregateBy: 'app' | |
| }; | |
| promises.push( | |
| Result.fromPromise(getFlowGenericMetrics(fqTLS, range)).then(res => { | |
| const tlsFlowRate = res | |
| .map(success => | |
| success.metrics.length > 0 ? { ...success.metrics[0], name: t('TLS flows') } : undefined | |
| ) | |
| .mapError(err => getStructuredHTTPError(err, `TLS flow rate`)); | |
| currentMetrics = { ...currentMetrics, tlsFlowRate }; | |
| setMetrics(currentMetrics); | |
| return res.map(r => r.stats).or(emptyStats); | |
| }) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/components/tabs/netflow-overview/netflow-overview.tsx` around lines
410 - 435, The metric legend labels are hardcoded in the Result handlers for
getFlowGenericMetrics — replace the literal strings 'Total flows' and 'TLS
flows' with translated strings using the react-i18next t(...) function (i.e. set
totalFlowRate to use name: t('Total flows') and tlsFlowRate to use name: t('TLS
flows')), ensure the component has access to t (via useTranslation) and add t to
the fetch useCallback dependency array so setMetrics/currentMetrics updates use
the localized labels correctly.
Member
|
/label qe-approved |
6 tasks
Member
|
@jotak There are some coderabbit review comments, do you want to fix them before merging? |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1530 +/- ##
==========================================
- Coverage 52.42% 52.22% -0.20%
==========================================
Files 233 233
Lines 12452 12551 +99
Branches 1564 1575 +11
==========================================
+ Hits 6528 6555 +27
- Misses 5310 5378 +68
- Partials 614 618 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
Bot
removed
the
ok-to-test
Member
|
@Amoghrd it's nothing big but yes i've addressed some of the feedback |
jotak
added
the
ok-to-test
|
New image: quay.io/netobserv/network-observability-console-plugin:65df4bc
It will expire in two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=65df4bc make set-plugin-image |
jotak
changed the title
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This is an automated cherry-pick of #1478
/assign Amoghrd
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests