Update flow model doc #383

jotak · 2023-09-11T07:13:37Z

Regenerating netflows model API doc (that's the first step, another PR will follow on operator repo)

codecov · 2023-09-11T07:20:25Z

Codecov Report

Patch coverage: 93.25% and project coverage change: -0.37% ⚠️

Comparison is base (bffb860) 57.69% compared to head (af3964c) 57.33%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #383      +/-   ##
==========================================
- Coverage   57.69%   57.33%   -0.37%     
==========================================
  Files         166      168       +2     
  Lines        7817     7936     +119     
  Branches      962      968       +6     
==========================================
+ Hits         4510     4550      +40     
- Misses       3030     3107      +77     
- Partials      277      279       +2

Flag	Coverage Δ
uitests	`58.24% <93.25%> (+0.22%)`	⬆️
unittests	`54.79% <ø> (-1.98%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
web/src/api/ipfix.ts	`100.00% <ø> (ø)`
web/src/model/filters.ts	`78.46% <ø> (ø)`
web/src/utils/filter-definitions.ts	`66.66% <ø> (ø)`
web/src/components/netflow-traffic.tsx	`55.24% <80.00%> (-0.04%)`	⬇️
web/src/utils/back-and-forth.ts	`94.73% <94.73%> (ø)`
web/src/utils/metrics.ts	`88.16% <94.87%> (+1.80%)`	⬆️
web/src/model/flow-query.ts	`87.50% <100.00%> (-10.07%)`	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

OlivierCazade

LGTM

skrthomas · 2023-09-11T20:27:42Z

web/docs/interfaces/Fields.md

@@ -218,6 +354,14 @@ Timestamp when this flow was received and processed by the flow collector, in se

 ___

+### TimeFlowRttNs


Will RTT be included in the product docs regeneration? 🤔

Yes, every field is present here even if it's feature related. However we may add a mention like *Dev Preview* in between to clarify it.

skrthomas · 2023-09-11T20:28:22Z

web/docs/interfaces/Fields.md

+
+• `Optional` **PktDropPackets**: `number`
+
+Number of packets dropped by the kernel in this flow


Suggested change

Number of packets dropped by the kernel in this flow

Number of packets dropped by the kernel in a specific flow

I'm not sure about that: all those fields are related to a given flow, hence the this. If I read in a specific flow then I would wonder: which flow are we talking about? Whereas this is contextual. I think I can just remove the in this flow part, as it isn't ambiguous anyway.

If the in this flow part can be removed that may be best because when I read this I was also wondering which flow is this? I can see how it may be more clear in context though, like you say.

skrthomas · 2023-09-11T20:28:35Z

web/docs/interfaces/Fields.md

+
+• `Optional` **PktDropBytes**: `number`
+
+Number of bytes dropped by the kernel in this flow


Suggested change

Number of bytes dropped by the kernel in this flow

Number of bytes dropped by the kernel in a specific flow

skrthomas · 2023-09-11T20:29:30Z

web/docs/enums/InterfaceDirection.md

+
+• **Ingress** = ``"0"``
+
+Incoming traffic, from network interface observation point


Suggested change

Incoming traffic, from network interface observation point

Incoming traffic, from the network interface observation point

skrthomas · 2023-09-11T20:29:40Z

web/docs/enums/InterfaceDirection.md

+
+• **Egress** = ``"1"``
+
+Outgoing traffic, from network interface observation point


Suggested change

Outgoing traffic, from network interface observation point

Outgoing traffic, from the network interface observation point

skrthomas · 2023-09-11T20:30:23Z

web/docs/enums/FlowDirection.md

+
+• **Inner** = ``"2"``
+
+Inner traffic, ie. same source and destination node


Suggested change

Inner traffic, ie. same source and destination node

Inner traffic, such as source and destination node

IBM Style Guide suggests to always use the text "for example" or "such as". Never use the phrase "for instance" or the Latin abbreviations e.g. or i.e. Can we avoid i.e.?

here this isn't an example, but a clarification, so "such as" doesn't work, need to find something else

btw I was wondering if that's "node" or "nodes" here ... bc we say "A and B" so that should be plural, except that A is B so it's singular? :-)

skrthomas

I left a few comments :)

jotak · 2023-09-12T08:33:42Z

thanks @skrthomas , PR updated, better like that?

skrthomas · 2023-09-12T14:41:35Z

web/docs/enums/FlowDirection.md

+
+• **Inner** = ``"2"``
+
+Inner traffic, with same source and destination node


Suggested change

Inner traffic, with same source and destination node

Inner traffic, with source and destination traffic in the same node

I think singular node is what we want here for sure. Alternatively, maybe just this:

Inner traffic, with the same source and destination node

skrthomas · 2023-09-12T14:44:24Z

@jotak those updates look good. I left one small note about inner traffic mostly just requiring a "the" before "same" but otherwise, lgtm.

jotak · 2023-09-12T16:44:13Z

thanks @skrthomas , done!

skrthomas

/lgtm

jotak · 2023-09-14T15:59:39Z

/approve

openshift-ci · 2023-09-14T15:59:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jotak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jotak]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Update flow model doc

bf92d4c

jotak requested review from OlivierCazade, skrthomas and jpinsonneau September 11, 2023 07:13

OlivierCazade previously approved these changes Sep 11, 2023

View reviewed changes

openshift-ci bot assigned OlivierCazade Sep 11, 2023

openshift-ci bot added the lgtm label Sep 11, 2023

skrthomas reviewed Sep 11, 2023

View reviewed changes

address feedback

8663bad

jotak dismissed OlivierCazade’s stale review via 8663bad September 12, 2023 08:32

openshift-ci bot removed the lgtm label Sep 12, 2023

skrthomas reviewed Sep 12, 2023

View reviewed changes

Address feedback: 'the' same

af3964c

jpinsonneau approved these changes Sep 14, 2023

View reviewed changes

openshift-ci bot assigned jpinsonneau Sep 14, 2023

openshift-ci bot added the lgtm label Sep 14, 2023

skrthomas approved these changes Sep 14, 2023

View reviewed changes

openshift-ci bot assigned skrthomas Sep 14, 2023

openshift-ci bot added the approved label Sep 14, 2023

openshift-merge-robot merged commit c5eaa19 into netobserv:main Sep 14, 2023
9 checks passed

jotak mentioned this pull request Sep 14, 2023

Consolidate flow json spec netobserv/network-observability-operator#417

Merged

10 tasks

jotak deleted the update-doc branch September 18, 2023 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update flow model doc #383

Update flow model doc #383

jotak commented Sep 11, 2023

codecov bot commented Sep 11, 2023 •

edited

Loading

OlivierCazade left a comment

skrthomas Sep 11, 2023

jpinsonneau Sep 12, 2023

skrthomas Sep 11, 2023

jotak Sep 12, 2023

skrthomas Sep 12, 2023

skrthomas Sep 11, 2023

skrthomas Sep 11, 2023

skrthomas Sep 11, 2023

skrthomas Sep 11, 2023 •

edited

Loading

skrthomas Sep 11, 2023 •

edited

Loading

jotak Sep 12, 2023

jotak Sep 12, 2023

skrthomas left a comment

jotak commented Sep 12, 2023

skrthomas Sep 12, 2023

skrthomas commented Sep 12, 2023

jotak commented Sep 12, 2023

skrthomas left a comment

jotak commented Sep 14, 2023

openshift-ci bot commented Sep 14, 2023

		@@ -218,6 +354,14 @@ Timestamp when this flow was received and processed by the flow collector, in se

		___

		### TimeFlowRttNs


		• `Optional` PktDropPackets: `number`

		Number of packets dropped by the kernel in this flow


		• `Optional` PktDropBytes: `number`

		Number of bytes dropped by the kernel in this flow


		• Ingress = ``"0"``

		Incoming traffic, from network interface observation point

	Incoming traffic, from network interface observation point
	Incoming traffic, from the network interface observation point


		• Egress = ``"1"``

		Outgoing traffic, from network interface observation point

	Outgoing traffic, from network interface observation point
	Outgoing traffic, from the network interface observation point


		• Inner = ``"2"``

		Inner traffic, ie. same source and destination node

	Inner traffic, ie. same source and destination node
	Inner traffic, such as source and destination node


		• Inner = ``"2"``

		Inner traffic, with same source and destination node

	Inner traffic, with same source and destination node
	Inner traffic, with source and destination traffic in the same node

Update flow model doc #383

Update flow model doc #383

Conversation

jotak commented Sep 11, 2023

codecov bot commented Sep 11, 2023 • edited Loading

Codecov Report

OlivierCazade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skrthomas Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

skrthomas Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skrthomas left a comment

Choose a reason for hiding this comment

jotak commented Sep 12, 2023

Choose a reason for hiding this comment

skrthomas commented Sep 12, 2023

jotak commented Sep 12, 2023

skrthomas left a comment

Choose a reason for hiding this comment

jotak commented Sep 14, 2023

openshift-ci bot commented Sep 14, 2023

codecov bot commented Sep 11, 2023 •

edited

Loading

skrthomas Sep 11, 2023 •

edited

Loading

skrthomas Sep 11, 2023 •

edited

Loading