Gather BIT metrics [implementation] #3122

originalsouth · 2024-06-20T17:03:01Z

QA notes

Please add some information for QA on how to test the newly created code.

Code Checklist

All the commits in this PR are properly PGP-signed and verified.
This PR only contains functionality relevant to the issue.
I have written unit tests for the changes or fixes I made.
I have checked the documentation and made changes where necessary.
I have performed a self-review of my code and refactored it to the best of my abilities.

Tickets have been created for newly discovered issues.
For any non-trivial functionality, I have added integration and/or end-to-end tests.
I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.

Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

ammar92 · 2024-06-25T07:21:33Z

Quick question: this is supposed to be used during development and not as a publicly available feature flag, right?

ammar92

Nice work, this will help us improve performance and provide valuable insights 👍 I do have a few questions and remarks to improve this even more.

Also, I still think we should eventually use OpenTelemetry for these kind of benchmarks or metrics gathering. It gives a more standardize way to deal with this and possibly save us a lot of code and effort in developing our own tools. This metric isn't the best example maybe, but it's close to what is possible with OpenTelemetry.

octopoes/tools/analyze-bit-metric.py

ammar92 · 2024-06-28T15:57:38Z

octopoes/octopoes/config/settings.py

@@ -89,3 +89,4 @@ def settings_customise_sources(
 DEFAULT_LIMIT = 50
 DEFAULT_OFFSET = 0
 QUEUE_NAME_OCTOPOES: str = "octopoes"
+GATHER_BIT_METRICS: bool = False


I'm not sure what the intention is, but usually feature flags are configurable. Perhaps this fits better in the Settings class and documented. This could work if it's only a private debug variable for development, although the location is bit misleading since it's placed next to the constants.

The bool hint is unnecessary here

The intention is to have a developer flag to toggle gathering [bit] metrics (without exposing it to the public). Please let me know what the best way of defining this would be.

octopoes/octopoes/core/service.py

originalsouth · 2024-07-04T09:40:35Z

Thanks @ammar92 for your review. I will address your style and efficiency concerns (whilst resolving the merge conflicts).

Regarding OpenTelemetry, can you elaborate on what you would like to see; where you would like this to go?

ammar92 · 2024-07-04T09:48:06Z

Thanks @ammar92 for your review. I will address your style and efficiency concerns (whilst resolving the merge conflicts).

Thanks and welcome back!

Regarding OpenTelemetry, can you elaborate on what you would like to see; where you would like this to go?

Let's meet soon to discuss this. The main message I'd like to convey is that it could provide us with valuable insights into performance, but also how the application functions over time or the scoped timeline of a specific action within the application (e.g. what requests were made, in what order and consumed time).

stephanie0x00 · 2024-07-04T11:59:19Z

@originalsouth Could you also rename the branch from 'feature/3116' to something more descriptive? This will make typing release notes much easier. Thanks!

originalsouth · 2024-07-04T13:40:41Z

@originalsouth Could you also rename the branch from 'feature/3116' to something more descriptive? This will make typing release notes much easier. Thanks!

We had this discussion before, branch names do not matter whatsoever and make a whole mess when changing. I can rename it if you insist but I do not see the point.

underdarknl · 2024-07-09T08:12:28Z

@originalsouth Could you also rename the branch from 'feature/3116' to something more descriptive? This will make typing release notes much easier. Thanks!

We had this discussion before, branch names do not matter whatsoever and make a whole mess when changing. I can rename it if you insist but I do not see the point.

Lets keep our branch-names somewhat descriptive. They do matter in the creation of release notes, and when looking at the lists of branches. having said that, there's no need to rename this branch, as I agree that will just make things messy.

stephanie0x00 · 2024-07-11T09:28:11Z

Checklist for QA:

I have checked out this branch, and successfully ran a fresh make reset.
I confirmed that there are no unintended functional regressions in this branch:
- I have managed to pass the onboarding flow
- Objects and Findings are created properly
- Tasks are created and completed properly
I confirmed that the PR's advertised feature or hotfix works as intended.
I checked the logs for errors and/or warnings and made issues where necessary

What works:

Ran into some issues while running the scripts, which resulted in a few changes. Those are now fixed and it looks great. Only thing that remains is writing documentation on how to use the scripts. This will be picked up in #3228

What doesn't work:

n/a

Bug or feature?:

n/a

* main: (31 commits) Refactor Task List and filters with error handlers for Scheduler (#1957) Fix filtering on plugin_id for normalizers (#3226) Implement `structlog` (#3175) Gather BIT metrics [implementation] (#3122) Add observation data to observation table in OOI detail page (#3186) cve-2024-6387 from RickGeex (#3194) Recalculate bit when a config object changes (#3206) Use more concise regexes (#3181) Updated Django (#3217) Updated `zipp` (#3215) Feature/boefje normalizer config models (#3118) Updated `certifi` (#3209) Add pluginToggler.js to Aggregate Report (#3202) Update to Django 5.0 (#2939) Update Dockerfile, fix Sonarcloud issue (#3180) Better default list of world writable domains in CSP checker (#3165) Update 1.16 release notes (#3195) Remove non standard header findings and add deprecated headers findings (#3127) Fix/sonarcloud https redirect dockerfiles (#3185) Bump docker/build-push-action from 5 to 6 (#3164) ...

Inflate 3116

ad5fe37

originalsouth requested a review from a team as a code owner June 20, 2024 17:03

originalsouth added octopoes Issues related to octopoes backend bits performance labels Jun 20, 2024

originalsouth added 5 commits June 20, 2024 19:05

Set default off

aa70bf7

Add yielded oois

33139a6

Prevent bytes-strings

79a94bf

Merge remote-tracking branch 'origin/main' into feature/3116

5d50bfe

Add bit metric "parser"

54cea93

ammar92 reviewed Jun 28, 2024

View reviewed changes

madelondohmen assigned originalsouth Jul 4, 2024

Manual merge of origin/main and address comments by @ammer92

90fbda2

originalsouth force-pushed the feature/3116 branch from 3851bf7 to 90fbda2 Compare July 4, 2024 15:11

originalsouth and others added 5 commits July 5, 2024 11:29

Remove annotations from settings

030ec3d

Merge remote-tracking branch 'origin/main' into feature/3116

059c461

Merge remote-tracking branch 'origin/main' into feature/3116

8216ab6

Merge remote-tracking branch 'origin/main' into feature/3116

d5c3d1b

Merge branch 'main' into feature/3116

2bc2ead

originalsouth added 4 commits July 9, 2024 16:29

Merge remote-tracking branch 'origin/main' into feature/3116

7ee2621

Merge remote-tracking branch 'origin/main' into feature/3116

aa74603

Merge remote-tracking branch 'origin/main' into feature/3116

2912965

Merge remote-tracking branch 'origin/main' into feature/3116

f70c5c3

originalsouth added 2 commits July 10, 2024 14:50

Merge remote-tracking branch 'origin/main' into feature/3116

2430f64

Merge remote-tracking branch 'origin/main' into feature/3116

2ab00ca

stephanie0x00 mentioned this pull request Jul 11, 2024

Documentation - Gather bit metrics #3228

Closed

originalsouth added 2 commits July 11, 2024 11:18

Make analyze-bit-metric.py more user friendly

89c9806

Merge remote-tracking branch 'origin/main' into feature/3116

6dc35e5

underdarknl approved these changes Jul 11, 2024

View reviewed changes

underdarknl merged commit 9b38852 into main Jul 11, 2024
20 checks passed

underdarknl deleted the feature/3116 branch July 11, 2024 09:34

originalsouth restored the feature/3116 branch July 29, 2024 14:07

originalsouth deleted the feature/3116 branch July 29, 2024 14:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gather BIT metrics [implementation] #3122

Gather BIT metrics [implementation] #3122

originalsouth commented Jun 20, 2024

ammar92 commented Jun 25, 2024

ammar92 left a comment

ammar92 Jun 28, 2024

originalsouth Jul 4, 2024

originalsouth commented Jul 4, 2024

ammar92 commented Jul 4, 2024

stephanie0x00 commented Jul 4, 2024

originalsouth commented Jul 4, 2024 •

edited

Loading

underdarknl commented Jul 9, 2024 •

edited

Loading

stephanie0x00 commented Jul 11, 2024

Gather BIT metrics [implementation] #3122

Gather BIT metrics [implementation] #3122

Conversation

originalsouth commented Jun 20, 2024

Changes

Issue link

Demo

QA notes

Code Checklist

Checklist for code reviewers:

Checklist for QA:

ammar92 commented Jun 25, 2024

ammar92 left a comment

Choose a reason for hiding this comment

ammar92 Jun 28, 2024

Choose a reason for hiding this comment

originalsouth Jul 4, 2024

Choose a reason for hiding this comment

originalsouth commented Jul 4, 2024

ammar92 commented Jul 4, 2024

stephanie0x00 commented Jul 4, 2024

originalsouth commented Jul 4, 2024 • edited Loading

underdarknl commented Jul 9, 2024 • edited Loading

stephanie0x00 commented Jul 11, 2024

Checklist for QA:

What works:

What doesn't work:

Bug or feature?:

originalsouth commented Jul 4, 2024 •

edited

Loading

underdarknl commented Jul 9, 2024 •

edited

Loading