Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 8 additions & 4 deletions config/sample-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,7 @@ frontend:
description: Source namespace
- name: SrcAddr
type: string
format: IP
description: Source IP address (ipv4 or ipv6)
- name: SrcPort
type: number
Expand All @@ -1232,6 +1233,7 @@ frontend:
description: Source MAC address
- name: SrcK8S_HostIP
type: string
format: IP
description: Source node IP
- name: SrcK8S_HostName
type: string
Expand Down Expand Up @@ -1259,6 +1261,7 @@ frontend:
description: Destination namespace
- name: DstAddr
type: string
format: IP
description: Destination IP address (ipv4 or ipv6)
- name: DstPort
type: number
Expand All @@ -1268,6 +1271,7 @@ frontend:
description: Destination MAC address
- name: DstK8S_HostIP
type: string
format: IP
description: Destination node IP
- name: DstK8S_HostName
type: string
Expand Down Expand Up @@ -1304,16 +1308,16 @@ frontend:
- 1: Egress (outgoing traffic, from the node observation point) +
- 2: Inner (with the same source and destination node)
- name: IfDirections
type: number
type: number array
Copy link
Member

Choose a reason for hiding this comment

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

in another PR ( https://github.com/netobserv/network-observability-operator/pull/999/files#diff-1fc45b6b65862012ab2d4a19e8f92ce31c1801f0ed7c464c13ac3f0dc5a3b756R1268 - which you can review btw ;-) ), I'm doing the same kind of thing, except I use a new field name to avoid breaking something in the console. So, I guess it means I can rollback that part.
Just, I used the brackets convention to describe an array, like number[] ; wdyt, I'd say it's sufficiently known convention so we can use it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change the array keyword to [] indeed 👌

Copy link
Member

@jotak jotak Jan 20, 2025

Choose a reason for hiding this comment

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

an advantage of [] is that it takes less space here https://docs.openshift.com/container-platform/4.17/observability/network_observability/json-flows-format-reference.html
On my screen, the table isn't displayed full-width (also because of the TOC column) - IMO it could be better to keep it small

Copy link
Member

Choose a reason for hiding this comment

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

although in the API reference
image
it's "array (string)", so yet something else. Honestly, I don't care too much :-) It's just that string[] felt more natural to me, of course I have my developer bias

Copy link
Contributor Author

Choose a reason for hiding this comment

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

description: |
Flow directions from the network interface observation point. Can be one of: +
- 0: Ingress (interface incoming traffic) +
- 1: Egress (interface outgoing traffic)
- name: Interfaces
type: string
type: string array
description: Network interfaces
- name: Flags
type: string
type: string array
description: |
Logical OR combination of unique TCP flags comprised in the flow, as per RFC-9293, with additional custom flags to represent the following per-packet combinations: +
- SYN+ACK (0x100) +
Expand Down Expand Up @@ -1361,7 +1365,7 @@ frontend:
type: number
description: TCP Smoothed Round Trip Time (SRTT), in nanoseconds
- name: NetworkEvents
type: string
type: string array
description: Network events flow monitoring
- name: K8S_ClusterName
type: string
Expand Down
7 changes: 7 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ type QuickFilter struct {
type FieldConfig struct {
Name string `yaml:"name" json:"name"`
Type string `yaml:"type" json:"type"`
Format string `yaml:"format,omitempty" json:"format,omitempty"`
Description string `yaml:"description" json:"description"`
// lokiLabel flag is for documentation only. Use loki.labels instead
Filter string `yaml:"filter,omitempty" json:"filter,omitempty"`
Expand Down Expand Up @@ -218,6 +219,12 @@ func ReadFile(version, date, filename string) (*Config, error) {
if cfg.IsLokiEnabled() {
cfg.Frontend.DataSources = append(cfg.Frontend.DataSources, string(constants.DataSourceLoki))
cfg.Frontend.LokiMocks = cfg.Loki.UseMocks
cfg.Loki.FieldsType = make(map[string]string)
cfg.Loki.FieldsFormat = make(map[string]string)
for _, f := range cfg.Frontend.Fields {
cfg.Loki.FieldsType[f.Name] = f.Type
cfg.Loki.FieldsFormat[f.Name] = f.Format
}
}

if cfg.IsPromEnabled() {
Expand Down
56 changes: 41 additions & 15 deletions pkg/config/loki.go
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
package config

import "github.com/netobserv/network-observability-console-plugin/pkg/utils"
import (
"fmt"
"strings"

"github.com/netobserv/network-observability-console-plugin/pkg/utils"
)

type Loki struct {
URL string `yaml:"url" json:"url"`
Labels []string `yaml:"labels" json:"labels"`
StatusURL string `yaml:"statusUrl,omitempty" json:"statusUrl,omitempty"`
Timeout Duration `yaml:"timeout,omitempty" json:"timeout,omitempty"`
TenantID string `yaml:"tenantID,omitempty" json:"tenantID,omitempty"`
TokenPath string `yaml:"tokenPath,omitempty" json:"tokenPath,omitempty"`
SkipTLS bool `yaml:"skipTls,omitempty" json:"skipTls,omitempty"`
CAPath string `yaml:"caPath,omitempty" json:"caPath,omitempty"`
StatusSkipTLS bool `yaml:"statusSkipTls,omitempty" json:"statusSkipTls,omitempty"`
StatusCAPath string `yaml:"statusCaPath,omitempty" json:"statusCaPath,omitempty"`
StatusUserCertPath string `yaml:"statusUserCertPath,omitempty" json:"statusUserCertPath,omitempty"`
StatusUserKeyPath string `yaml:"statusUserKeyPath,omitempty" json:"statusUserKeyPath,omitempty"`
UseMocks bool `yaml:"useMocks,omitempty" json:"useMocks,omitempty"`
ForwardUserToken bool `yaml:"forwardUserToken,omitempty" json:"forwardUserToken,omitempty"`
URL string `yaml:"url" json:"url"`
Labels []string `yaml:"labels" json:"labels"`
FieldsType map[string]string `yaml:"fieldsType" json:"fieldsType"`
FieldsFormat map[string]string `yaml:"fieldsFormat" json:"fieldsFormat"`
StatusURL string `yaml:"statusUrl,omitempty" json:"statusUrl,omitempty"`
Timeout Duration `yaml:"timeout,omitempty" json:"timeout,omitempty"`
TenantID string `yaml:"tenantID,omitempty" json:"tenantID,omitempty"`
TokenPath string `yaml:"tokenPath,omitempty" json:"tokenPath,omitempty"`
SkipTLS bool `yaml:"skipTls,omitempty" json:"skipTls,omitempty"`
CAPath string `yaml:"caPath,omitempty" json:"caPath,omitempty"`
StatusSkipTLS bool `yaml:"statusSkipTls,omitempty" json:"statusSkipTls,omitempty"`
StatusCAPath string `yaml:"statusCaPath,omitempty" json:"statusCaPath,omitempty"`
StatusUserCertPath string `yaml:"statusUserCertPath,omitempty" json:"statusUserCertPath,omitempty"`
StatusUserKeyPath string `yaml:"statusUserKeyPath,omitempty" json:"statusUserKeyPath,omitempty"`
UseMocks bool `yaml:"useMocks,omitempty" json:"useMocks,omitempty"`
ForwardUserToken bool `yaml:"forwardUserToken,omitempty" json:"forwardUserToken,omitempty"`
labelsMap map[string]struct{}
}

Expand All @@ -34,3 +41,22 @@ func (l *Loki) IsLabel(key string) bool {
_, isLabel := l.labelsMap[key]
return isLabel
}

func (l *Loki) IsNumeric(v string) bool {
// check on Field / SrcField / DstField since we remove prefix in some cases for common filtering
types := fmt.Sprintf("%s|%s|%s", l.FieldsType[v], l.FieldsType["Src"+v], l.FieldsType["Dst"+v])
return strings.Contains(types, "number")
}

func (l *Loki) IsIP(v string) bool {
// check on Field / SrcField / DstField since we remove prefix in some cases for common filtering
formats := fmt.Sprintf("%s|%s|%s", l.FieldsFormat[v], l.FieldsFormat["Src"+v], l.FieldsFormat["Dst"+v])
return strings.Contains(formats, "IP")
}

func (l *Loki) IsArray(v string) bool {
// check on Field / SrcField / DstField since we remove prefix in some cases for common filtering
types := fmt.Sprintf("%s|%s|%s", l.FieldsType[v], l.FieldsType["Src"+v], l.FieldsType["Dst"+v])
return strings.Contains(types, "array")

}
6 changes: 3 additions & 3 deletions pkg/loki/flow_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (q *FlowQueryBuilder) addFilter(filter filters.Match) error {
if lf, ok := filter.ToLabelFilter(); ok {
q.labelFilters = append(q.labelFilters, lf)
}
} else if fields.IsIP(filter.Key) {
} else if q.config.IsIP(filter.Key) {
q.addIPFilters(filter.Key, values, filter.Not)
} else {
q.addLineFilters(filter.Key, values, filter.Not, filter.MoreThanOrEqual)
Expand All @@ -133,12 +133,12 @@ func (q *FlowQueryBuilder) addLineFilters(key string, values []string, not bool,
return
}

if fields.IsArray(key) {
if q.config.IsArray(key) {
q.lineFilters = append(q.lineFilters, filters.ArrayLineFilter(key, values, not))
} else {
var lf filters.LineFilter
var hasEmptyMatch bool
if fields.IsNumeric(key) {
if q.config.IsNumeric(key) {
lf, hasEmptyMatch = filters.NumericLineFilter(key, values, not, moreThan)
} else {
lf, hasEmptyMatch = filters.StringLineFilterCheckExact(key, values, not)
Expand Down
51 changes: 0 additions & 51 deletions pkg/model/fields/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,54 +60,3 @@ const (
XlatDstAddr = "XlatDstAddr"
XlatZoneID = "ZoneId"
)

func IsNumeric(v string) bool {
switch v {
case
DNSID,
DNSLatency,
DNSErrNo,
TimeFlowRTT,
Port,
SrcPort,
DstPort,
Packets,
Proto,
Bytes,
DSCP,
XlatDstPort,
XlatSrcPort,
XlatZoneID:
return true
default:
return false
}
}

func IsIP(f string) bool {
switch f {
case
DstAddr,
SrcAddr,
DstHostIP,
SrcHostIP,
XlatDstAddr,
XlatSrcAddr:
return true
default:
return false
}
}

func IsArray(v string) bool {
switch v {
case
IfDirections,
Interfaces,
NetworkEvents,
TCPFlags:
return true
default:
return false
}
}
9 changes: 9 additions & 0 deletions pkg/server/server_flows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,15 @@ func TestLokiFiltering(t *testing.T) {
Loki: config.Loki{
URL: lokiSvc.URL,
Labels: []string{"SrcK8S_Namespace", "SrcK8S_OwnerName", "DstK8S_Namespace", "DstK8S_OwnerName", "FlowDirection"},
FieldsType: map[string]string{
"Proto": "number",
"SrcPort": "number",
"DstPort": "number",
},
FieldsFormat: map[string]string{
"SrcAddr": "IP",
"DstAddr": "IP",
},
},
Frontend: config.Frontend{Deduper: config.Deduper{Mark: true}},
}, &authM)
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/query-summary/summary-panel-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ export const SummaryPanelContent: React.FC<SummaryPanelContentProps> = ({
<>
{tc.objects
.sort((a, b) => compareStrings(a.namespace ? a.namespace : '', b.namespace ? b.namespace : ''))
.flatMap(o => (
<AccordionExpandedContentBody>
.flatMap((o, i) => (
<AccordionExpandedContentBody key={`expanded-content-body${i}`}>
{o.namespace && <ResourceLink key={`${tc.type}-${o.namespace}`} kind={'Namespace'} name={o.namespace} />}
{o.names
.sort((a, b) => compareStrings(a, b))
Expand Down
5 changes: 4 additions & 1 deletion web/src/utils/fields.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
export type FieldType = 'string' | 'number';
export type FieldType = 'string' | 'string array' | 'number' | 'number array';
Copy link
Member

Choose a reason for hiding this comment

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

isn't it going to break something in the forceType function? This was the concern I had in netobserv/network-observability-operator#999 and the reason why I introduced docType instead ...

Copy link
Member

Choose a reason for hiding this comment

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

(actually, reading the code again, maybe it was because I used object[] for net events that I had to introduce docType)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forceType is skipping arrays so it should be good for now:

const forceType = (id: ColumnsId, value: ColValue, type?: FieldType): ColValue => {
if (!type) {
console.error('Column ' + id + " doesn't specify type");
}
// check if value type match and convert it if needed
if (value && value !== '' && typeof value !== type && !Array.isArray(value)) {

We may improve it if needed. We could force arrays too 🤔


export type FieldFormat = 'IP';

export interface FieldConfig {
name: string;
type: FieldType;
format?: FieldFormat;
description: string;
filter?: string;
}
Loading