Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions config/sample-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -697,11 +697,11 @@ frontend:
filter: tls_cipher_suite
width: 15
feature: tlsTracking
- id: TLSCurve
- id: TLSGroup
group: Protocol Info
name: TLS Curve
field: TLSCurve
filter: tls_curve
name: TLS Group
field: TLSGroup
filter: tls_group
width: 10
feature: tlsTracking
- id: TLSTypes
Expand Down Expand Up @@ -1411,11 +1411,11 @@ frontend:
placeholder: 'E.g: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256'
hint: Specify a TLS cipher suite.
feature: tlsTracking
- id: tls_curve
name: TLS curve
- id: tls_group
name: TLS group
component: text
placeholder: 'E.g: X25519'
hint: Specify a TLS curve name.
hint: Specify a TLS group name.
feature: tlsTracking
- id: tls_types
name: TLS packet type
Expand Down Expand Up @@ -1837,9 +1837,9 @@ frontend:
- name: TLSCipherSuite
type: string
description: TLS cipher suite
- name: TLSCurve
- name: TLSGroup
type: string
description: TLS curve name
description: TLS group name
- name: Dscp
type: number
description: Differentiated Services Code Point (DSCP) value
Expand Down
39 changes: 21 additions & 18 deletions pkg/loki/flow_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,25 +126,24 @@ func (q *FlowQueryBuilder) addLineFilters(filter filters.Match, values []string)
return
}

if q.config.IsArray(filter.Key) {
q.lineFilters = append(q.lineFilters, filters.ArrayLineFilter(filter.Key, values, filter.Not))
var lf filters.LineFilter
var hasEmptyMatch bool
switch {
case q.config.IsArray(filter.Key):
lf, hasEmptyMatch = filters.ArrayLineFilter(filter.Key, values, filter.Not)
case q.config.IsNumeric(filter.Key):
lf, hasEmptyMatch = filters.NumericLineFilter(filter.Key, values, filter.Not, filter.MoreThanOrEqual)
case filter.Regex:
lf, hasEmptyMatch = filters.StringLineFilterCheckExact(filter.Key, values, filter.Not)
default:
lf, hasEmptyMatch = filters.StringLineFilter(filter.Key, values, filter.Not)
}
// if there is at least an empty exact match, there is no uniform/safe way to filter by text,
// so we should use JSON label matchers instead of text line matchers
if hasEmptyMatch {
q.jsonFilters = append(q.jsonFilters, lf.AsLabelFilters())
} else {
var lf filters.LineFilter
var hasEmptyMatch bool
if q.config.IsNumeric(filter.Key) {
lf, hasEmptyMatch = filters.NumericLineFilter(filter.Key, values, filter.Not, filter.MoreThanOrEqual)
} else if filter.Regex {
lf, hasEmptyMatch = filters.StringLineFilterCheckExact(filter.Key, values, filter.Not)
} else {
lf, hasEmptyMatch = filters.StringLineFilter(filter.Key, values, filter.Not)
}
// if there is at least an empty exact match, there is no uniform/safe way to filter by text,
// so we should use JSON label matchers instead of text line matchers
if hasEmptyMatch {
q.jsonFilters = append(q.jsonFilters, lf.AsLabelFilters())
} else {
q.lineFilters = append(q.lineFilters, lf)
}
q.lineFilters = append(q.lineFilters, lf)
}
}

Expand Down Expand Up @@ -225,6 +224,10 @@ func (q *FlowQueryBuilder) appendDNSFilter(sb *strings.Builder) {
sb.WriteString("`")
}

func (q *FlowQueryBuilder) appendTLSFilter(sb *strings.Builder) {
sb.WriteString(`|="TLSTypes"`)
}

func (q *FlowQueryBuilder) appendDNSLatencyFilter(sb *strings.Builder) {
// ensure DnsLatencyMs field is specified and value is not zero
// |~`"DnsLatencyMs`!~`DnsLatencyMs%22:0[,}]`
Expand Down
30 changes: 17 additions & 13 deletions pkg/loki/topology_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ type TopologyInput struct {
Groups string
}

func (in *TopologyInput) GetActualDataField() string {
switch in.DataField {
case constants.MetricTypeFlows, constants.MetricTypeDNSFlows, constants.MetricTypeTLSFlows:
return ""
default:
return in.DataField
}
}

type TopologyQueryBuilder struct {
*FlowQueryBuilder
topology *TopologyInput
Expand Down Expand Up @@ -71,15 +80,6 @@ func GetLabelsAndFilter(kl map[string][]string, aggregate, groups string) ([]str
return fields, filter
}

func getField(metricType string) string {
switch metricType {
case constants.MetricTypeFlows, constants.MetricTypeDNSFlows:
return ""
default:
return metricType
}
}

func getFactor(metricType string) string {
switch metricType {
case constants.MetricTypeFlowRTT:
Expand Down Expand Up @@ -124,7 +124,6 @@ func (q *TopologyQueryBuilder) Build() string {
}
strLabels := strings.Join(labels, ",")

dataField := getField(q.topology.DataField)
factor := getFactor(q.topology.DataField)
function, quantile := GetFunctionWithQuantile(q.topology.MetricFunction)

Expand Down Expand Up @@ -171,15 +170,20 @@ func (q *TopologyQueryBuilder) Build() string {
q.appendFilter(sb, extraFilter)
}

if dataField == constants.MetricTypeDNSLatency {
switch q.topology.DataField {
case constants.MetricTypeDNSLatency:
q.appendDNSLatencyFilter(sb)
} else if dataField == constants.MetricTypeDNSFlows {
case constants.MetricTypeDNSFlows:
q.appendDNSFilter(sb)
} else if dataField == constants.MetricTypeFlowRTT {
case constants.MetricTypeTLSFlows:
q.appendTLSFilter(sb)
case constants.MetricTypeFlowRTT:
q.appendRTTFilter(sb)
}

q.appendJSON(sb, true)

dataField := q.topology.GetActualDataField()
if len(dataField) > 0 {
sb.WriteString("|unwrap ")
sb.WriteString(dataField)
Expand Down
14 changes: 12 additions & 2 deletions pkg/model/filters/logql.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,22 @@ func NumericLineFilter(key string, values []string, not, moreThan bool) (LineFil
typeNumber)
}

func ArrayLineFilter(key string, values []string, not bool) LineFilter {
// ArrayLineFilter returns a LineFilter and true if it has an empty match
func ArrayLineFilter(key string, values []string, not bool) (LineFilter, bool) {
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
Comment on lines +276 to +289
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

}

// StringLineFilter returns a LineFilter and true if it has an empty match
Expand Down
6 changes: 6 additions & 0 deletions pkg/prometheus/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ func NewInventory(cfg *config.Prometheus) *Inventory {
cpy.ValueField = constants.MetricTypeDNSFlows
toAppend = append(toAppend, cpy)
}
// Simulate Flows and TLSFlows value fields
if strings.Contains(cfg.Metrics[i].Name, "_tls_flows") {
cfg.Metrics[i].ValueField = constants.MetricTypeTLSFlows
} else if strings.Contains(cfg.Metrics[i].Name, "_flows") {
cfg.Metrics[i].ValueField = constants.MetricTypeFlows
}
}
return &Inventory{metrics: append(cfg.Metrics, toAppend...)}
}
Expand Down
22 changes: 21 additions & 1 deletion pkg/prometheus/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,25 @@ import (
"testing"

"github.com/netobserv/network-observability-console-plugin/pkg/config"
"github.com/netobserv/network-observability-console-plugin/pkg/utils/constants"
"github.com/stretchr/testify/assert"
)

var configuredMetrics = []config.MetricInfo{
{
Enabled: true,
Name: "netobserv_metric_tls_flows_total",
Type: "Counter",
Direction: config.AnyDirection,
Labels: []string{"SrcK8S_Namespace", "DstK8S_Namespace"},
},
{
Enabled: false,
Name: "netobserv_metric_tls_flows_disabled",
Type: "Counter",
Direction: config.AnyDirection,
Labels: []string{"SrcK8S_Namespace", "DstK8S_Namespace"},
},
{
Enabled: true,
Name: "netobserv_metric_1",
Expand Down Expand Up @@ -81,14 +96,19 @@ func TestInventory_Search(t *testing.T) {
assert.Empty(t, search.Found)
assert.Empty(t, search.Candidates)

// Search flows metrics
// Search flows metrics (and TLS metric is skipped)
search = inv.Search([]string{"SrcK8S_Namespace", "DstK8S_Namespace"}, "")
assert.Equal(t, []string{"netobserv_metric_3"}, search.Found)
assert.Empty(t, search.Candidates)

search = inv.Search([]string{"SrcK8S_HostName", "DstK8S_HostName"}, "")
assert.Equal(t, []string{"netobserv_metric_3"}, search.Found)
assert.Empty(t, search.Candidates)

// Search TLS flows metrics
search = inv.Search([]string{"SrcK8S_Namespace", "DstK8S_Namespace"}, constants.MetricTypeTLSFlows)
assert.Equal(t, []string{"netobserv_metric_tls_flows_total"}, search.Found)
assert.Empty(t, search.Candidates)
}

func TestInventory_Search_RTT_Candidate(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions pkg/utils/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const (
MetricTypeDroppedPackets = "PktDropPackets"
MetricTypeDNSLatency = "DnsLatencyMs"
MetricTypeDNSFlows = "DnsFlows"
MetricTypeTLSFlows = "TlsFlows"
MetricTypeFlowRTT = "TimeFlowRttNs"
DefaultMetricType = MetricTypeBytes

Expand Down
11 changes: 11 additions & 0 deletions web/locales/en/plugin__netobserv-plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,8 @@
"internal": "internal",
"P": "P",
"Pps": "Pps",
"flows": "flows",
"fps": "fps",
"minimum": "minimum",
"maximum": "maximum",
"90th percentile": "90th percentile",
Expand Down Expand Up @@ -694,6 +696,15 @@
"Top {{limit}} DNS response code": "Top {{limit}} DNS response code",
"Total DNS response code": "Total DNS response code",
"The top DNS response code extracted from DNS response headers compared to total over the selected interval": "The top DNS response code extracted from DNS response headers compared to total over the selected interval",
"TLS usage": "TLS usage",
"TLS usage (network flows per second)": "TLS usage (network flows per second)",
"TLS traffic": "TLS traffic",
"TLS usage per version": "TLS usage per version",
"TLS per version (network flows per second)": "TLS per version (network flows per second)",
"TLS usage per group": "TLS usage per group",
"TLS per group (network flows per second)": "TLS per group (network flows per second)",
"TLS usage per cipher suite": "TLS usage per cipher suite",
"TLS per cipher suite (network flows per second)": "TLS per cipher suite (network flows per second)",
"rates": "rates",
"with total": "with total",
"Invalid custom panel id": "Invalid custom panel id"
Expand Down
16 changes: 13 additions & 3 deletions web/src/api/loki.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ export interface StreamResult {
values: string[][];
}

export class RecordsResult {
export interface RecordsResult {
records: Record[];
stats: Stats;
}

export class FlowMetricsResult {
export interface FlowMetricsResult {
metrics: TopologyMetrics[];
stats: Stats;
}

export class GenericMetricsResult {
export interface GenericMetricsResult {
metrics: GenericMetric[];
stats: Stats;
}
Expand Down Expand Up @@ -147,6 +147,11 @@ export type NetflowMetrics = {
totalDnsLatency: Result<TotalFunctionMetrics, StructuredError | string>;
totalDnsCount: Result<GenericMetric, StructuredError | string>;
totalRtt: Result<TotalFunctionMetrics, StructuredError | string>;
tlsUsagePerVersion: Result<GenericMetric[], StructuredError | string>;
tlsUsagePerCipher: Result<GenericMetric[], StructuredError | string>;
tlsUsagePerGroup: Result<GenericMetric[], StructuredError | string>;
tlsFlowRate: Result<GenericMetric, StructuredError | string>;
totalFlowRate: Result<GenericMetric, StructuredError | string>;
custom: Map<string, Result<TopologyMetrics[] | GenericMetric[], StructuredError | string>>;
totalCustom: Map<string, Result<TopologyMetrics | GenericMetric, StructuredError | string>>;
};
Expand All @@ -166,6 +171,11 @@ export const defaultNetflowMetrics: NetflowMetrics = {
totalDnsLatency: Result.empty(),
totalDnsCount: Result.empty(),
totalRtt: Result.empty(),
tlsUsagePerCipher: Result.empty(),
tlsUsagePerGroup: Result.empty(),
tlsUsagePerVersion: Result.empty(),
tlsFlowRate: Result.empty(),
totalFlowRate: Result.empty(),
custom: new Map(),
totalCustom: new Map()
};
Expand Down
11 changes: 8 additions & 3 deletions web/src/api/routes.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import axios from 'axios';
import { Config, defaultConfig } from '../model/config';
import { buildExportQuery } from '../model/export-query';
import { FlowQuery, FlowScope, isTimeMetric } from '../model/flow-query';
import { FlowQuery, FlowScope, isTimeMetric, StructuredFlowQuery, structuredToRawQuery } from '../model/flow-query';
import { ContextSingleton } from '../utils/context';
import { TimeRange } from '../utils/datetime';
import { parseGenericMetrics, parseTopologyMetrics } from '../utils/metrics';
Expand Down Expand Up @@ -73,7 +73,8 @@ export const queryPrometheusMetric = (query: string): Promise<unknown> => {
});
};

export const getExportFlowsURL = (params: FlowQuery, columns?: string[]): string => {
export const getExportFlowsURL = (q: StructuredFlowQuery, columns?: string[]): string => {
const params = structuredToRawQuery(q);
const exportQuery = buildExportQuery(params, columns);
return `${ContextSingleton.getHost()}/api/loki/export?${exportQuery}`;
};
Expand Down Expand Up @@ -169,7 +170,11 @@ export const getFlowMetrics = (params: FlowQuery, range: number | TimeRange): Pr
});
};

export const getFlowGenericMetrics = (params: FlowQuery, range: number | TimeRange): Promise<GenericMetricsResult> => {
export const getFlowGenericMetrics = (
q: StructuredFlowQuery,
range: number | TimeRange
): Promise<GenericMetricsResult> => {
const params = structuredToRawQuery(q);
return getFlowMetricsGeneric(params, res => {
return parseGenericMetrics(
res.result as RawTopologyMetrics[],
Expand Down
1 change: 1 addition & 0 deletions web/src/components/drawer/netflow-traffic-drawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ export const NetflowTrafficDrawer = React.forwardRef<NetflowTrafficDrawerHandle,
metrics={props.metrics}
loading={props.loading}
isDark={isDarkTheme}
filterDefinitions={caps.filterDefs}
resetDefaultFilters={getResetDefaultFiltersProp()}
clearFilters={getClearFiltersProp()}
truncateLength={props.overviewTruncateLength}
Expand Down
1 change: 0 additions & 1 deletion web/src/components/messages/empty.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export const Empty: React.FC<EmptyProps> = ({ showDetails, resetDefaultFilters,
getStatus(ContextSingleton.getForcedNamespace())
.then(status => {
if (isMounted) {
console.info('status result', status);
setStatus(status);
setStatusError(undefined);
}
Expand Down
1 change: 0 additions & 1 deletion web/src/components/messages/error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ export const ErrorComponent: React.FC<ErrorProps> = ({ title, error }) => {

getStatus(ContextSingleton.getForcedNamespace())
.then(status => {
console.info('status result', status);
setStatus(status);
setStatusError(undefined);
})
Expand Down
Loading
Loading