NETOBSERV-1231: Improve promencode API: scaling & filters #513

jotak · 2023-10-09T16:20:20Z

New ValueScale config to allow scale conversion, e.g. RTT ns to seconds
To avoid having reserved characters or words in filters, use a dedicated API instead: new setting Filter.Type (exact,presence,absence or regex)
Pre-compute regex by setting up a "predicate" function per filter

Breaking-change : metrics currently defining filters such as "FlowDirection: 0|2" must be adapted with the new Filter Type API

Description

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

openshift-ci · 2023-10-09T16:20:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from jotak. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

jotak · 2023-10-11T07:53:28Z

For info, I set no-qe as this has no visible change; however this PR will help us define better RTT metrics for NETOBSERV-1231

codecov · 2023-10-11T08:01:10Z

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Comparison is base (e3a97f1) 67.07% compared to head (288202f) 66.98%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #513      +/-   ##
==========================================
- Coverage   67.07%   66.98%   -0.09%     
==========================================
  Files          94       95       +1     
  Lines        6724     6779      +55     
==========================================
+ Hits         4510     4541      +31     
- Misses       1948     1973      +25     
+ Partials      266      265       -1

Flag	Coverage Δ
unittests	`66.98% <60.39%> (-0.09%)`	⬇️

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

Files	Coverage Δ
pkg/api/enum.go	`75.75% <ø> (ø)`
pkg/pipeline/encode/encode_prom.go	`77.92% <96.00%> (+0.57%)`	⬆️
pkg/api/encode_prom.go	`77.77% <0.00%> (-22.23%)`	⬇️
pkg/pipeline/encode/encode_prom_metric.go	`77.08% <77.08%> (ø)`
pkg/pipeline/transform/kubernetes/kubernetes.go	`17.94% <0.00%> (-1.20%)`	⬇️

... and 1 file with indirect coverage changes

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

jpinsonneau

Sounds good, just a small comment in the code. You also need to run make docs to update api.md file.

On top of that, can you please either put NETOBSERV-1231 in the title or create another Jira task for this ?

Thanks @jotak !

jpinsonneau · 2023-10-11T08:29:29Z

pkg/pipeline/encode/encode_prom_test.go

+	flows := make([]config.GenericMap, 1000)
+	for i := 0; i < 1000; i++ {


rename the function above then 🤓

openshift-ci-robot · 2023-10-16T10:20:22Z

openshift-ci-robot · 2023-11-07T08:40:38Z

@jotak: This pull request references NETOBSERV-1231 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 "4.15.0" version, but no target version was set.

In response to this:

New ValueScale config to allow scale conversion, e.g. RTT ns to seconds

To avoid having reserved characters or words in filters, use a dedicated API instead: new setting Filter.Type (exact,presence,absence or regex)

Pre-compute regex by setting up a "predicate" function per filter

Breaking-change : metrics currently defining filters such as "FlowDirection: 0|2" must be adapted with the new Filter Type API

Description

Dependencies

n/a

Checklist

If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.

Will this change affect NetObserv / Network Observability operator? If not, you can ignore the rest of this checklist.

Is this PR backed with a JIRA ticket? If so, make sure it is written as a title prefix (in general, PRs affecting the NetObserv/Network Observability product should be backed with a JIRA ticket - especially if they bring user facing changes).

Does this PR require product documentation?

If so, make sure the JIRA epic is labelled with "documentation" and provides a description relevant for doc writers, such as use cases or scenarios. Any required step to activate or configure the feature should be documented there, such as new CRD knobs.

Does this PR require a product release notes entry?

If so, fill in "Release Note Text" in the JIRA.

Is there anything else the QE team should know before testing? E.g: configuration changes, environment setup, etc.

If so, make sure it is described in the JIRA ticket.

QE requirements (check 1 from the list):

Standard QE validation, with pre-merge tests unless stated otherwise.

Regression tests only (e.g. refactoring with no user-facing change).

No QE (e.g. trivial change with high reviewer's confidence, or per agreement with the QE team).

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 kubernetes/test-infra repository.

- New ValueScale config to allow scale conversion, e.g. RTT ns to seconds - To avoid having reserved characters or words in filters, use a dedicated API instead: new setting Filter.Type (exact,presence,absence or regex) - Pre-compute regex by setting up a "predicate" function per filter

openshift-ci · 2023-11-08T11:40:39Z

New changes are detected. LGTM label has been removed.

github-actions · 2023-11-08T11:41:41Z

New image:
quay.io/netobserv/flowlogs-pipeline:bb46a3c

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=bb46a3c make set-flp-image

github-actions · 2023-11-08T11:46:35Z

New image:
quay.io/netobserv/flowlogs-pipeline:0ee3fde

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=0ee3fde make set-flp-image

Related to netobserv/flowlogs-pipeline#513

jotak requested review from OlivierCazade and jpinsonneau October 11, 2023 07:51

jotak added no-qe This PR doesn't necessitate QE approval no-doc This PR doesn't require documentation change on the NetObserv operator labels Oct 11, 2023

jpinsonneau reviewed Oct 11, 2023

View reviewed changes

jotak changed the title ~~Improve promencode API: scaling & filters~~ NETOBSERV-1231: Improve promencode API: scaling & filters Oct 16, 2023

openshift-ci-robot added the jira/valid-reference label Oct 16, 2023

jotak requested a review from jpinsonneau October 16, 2023 10:25

OlivierCazade approved these changes Oct 16, 2023

View reviewed changes

openshift-ci bot assigned OlivierCazade Oct 16, 2023

openshift-ci bot added the lgtm label Oct 16, 2023

jpinsonneau approved these changes Oct 19, 2023

View reviewed changes

openshift-ci bot assigned jpinsonneau Oct 19, 2023

jotak added the breaking-change This pull request has breaking changes. They should be described in PR description. label Nov 7, 2023

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 8, 2023

jotak added 2 commits November 8, 2023 12:40

Rename func, add doc

288202f

jotak force-pushed the improve-promencode branch from 29a38e1 to 288202f Compare November 8, 2023 11:40

openshift-ci bot removed the lgtm label Nov 8, 2023

github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 8, 2023

jotak added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Nov 8, 2023

jotak merged commit fac8a61 into netobserv:main Nov 8, 2023
6 of 7 checks passed

jotak added a commit to jotak/network-observability-operator that referenced this pull request Nov 8, 2023

Update for FLP API breaking change

fa593dd

Related to netobserv/flowlogs-pipeline#513

jotak mentioned this pull request Nov 8, 2023

Update for FLP API breaking change netobserv/network-observability-operator#487

Merged

openshift-merge-bot bot pushed a commit to netobserv/network-observability-operator that referenced this pull request Nov 8, 2023

Update for FLP API breaking change (#487)

10c8bad

Related to netobserv/flowlogs-pipeline#513

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NETOBSERV-1231: Improve promencode API: scaling & filters #513

NETOBSERV-1231: Improve promencode API: scaling & filters #513

jotak commented Oct 9, 2023 •

edited

Loading

openshift-ci bot commented Oct 9, 2023

jotak commented Oct 11, 2023 •

edited by openshift-ci bot

Loading

codecov bot commented Oct 11, 2023 •

edited

Loading

jpinsonneau left a comment

jpinsonneau Oct 11, 2023

jotak Oct 16, 2023

openshift-ci-robot commented Oct 16, 2023 •

edited by openshift-ci bot

Loading

Description

Dependencies

Checklist

openshift-ci-robot commented Nov 7, 2023 •

edited by openshift-ci bot

Loading

Description

Dependencies

Checklist

openshift-ci bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

		flows := make([]config.GenericMap, 1000)
		for i := 0; i < 1000; i++ {

NETOBSERV-1231: Improve promencode API: scaling & filters #513

NETOBSERV-1231: Improve promencode API: scaling & filters #513

Conversation

jotak commented Oct 9, 2023 • edited Loading

Description

Dependencies

Checklist

openshift-ci bot commented Oct 9, 2023

jotak commented Oct 11, 2023 • edited by openshift-ci bot Loading

codecov bot commented Oct 11, 2023 • edited Loading

Codecov Report

jpinsonneau left a comment

Choose a reason for hiding this comment

jpinsonneau Oct 11, 2023

Choose a reason for hiding this comment

jotak Oct 16, 2023

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 16, 2023 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

openshift-ci-robot commented Nov 7, 2023 • edited by openshift-ci bot Loading

Description

Dependencies

Checklist

openshift-ci bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

github-actions bot commented Nov 8, 2023

jotak commented Oct 9, 2023 •

edited

Loading

jotak commented Oct 11, 2023 •

edited by openshift-ci bot

Loading

codecov bot commented Oct 11, 2023 •

edited

Loading

openshift-ci-robot commented Oct 16, 2023 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 7, 2023 •

edited by openshift-ci bot

Loading