[POA-2928] Changed the event watcher to watch pod changes not event changes #82

mudit-postman · 2025-02-18T11:57:23Z

Change from event watcher to pod watcher in k8s
Changes in k8s event worker, also added modified state handler
Remove state change to traffic stopped from traffic ended and errored
Change inspectPodForEnvVars func, it will now except podArgs object
Change the state management of pod traffic monitoring, state diagram
Changes in dump log formatting

- Add logs before each process start - Move default interval time to daemonset module:

- Handle case if any state change is allowed in changePodTrafficMonitorState() - if condition in addPodArgsToMap() is opposite also not this func will return error

…rt apidump process in next healthcheck run

- telemetry api erroring our due to nil resp var - already deleted pod is not getting removed during pod health check

…bs/postman-insights-agent into mudit/poa-2609-testing-changes

- Change from event watcher to pod watcher in k8s - Changes in k8s event worker, also added modified state handler - Remove state change to traffic stopped from traffic ended and errored - Change inspectPodForEnvVars func, it will now except podArgs object

- Added sync.WaitGroup for apidump processes - Refactored states to make them same as k8s pods states

Before: ``` [INFO] Running learn mode on interfaces lo, wlan0 [INFO] Send SIGINT (Ctrl-C) to stop... ^C[INFO] Received interrupt, stopping trace collection... panic: close of closed channel ``` After: ``` [INFO] Running learn mode on interfaces lo, wlan0 [INFO] Send SIGINT (Ctrl-C) to stop... ^C[INFO] Received interrupt, stopping trace collection... [INFO] Trace collection stopped [INFO] Did not capture any TCP packets matching the filter. This may mean your filter is incorrect, such as the wrong TCP port. [ERROR] No HTTP calls captured! 🛑 [ERROR] Error during initiaization: API trace is empty [INFO] This process will not exit, to avoid boot loops. Please correct the command line flags or environment and retry. ```

…into mudit/event-watcher-changes

mgritter

Looks OK, let's clean up the use of a bool for the double-close fix.

It would be nicer to identify what the underlying cause is to make sure there's not some other logic change -- it should be unambiguous whose job it is to stop the background process.

mgritter · 2025-02-19T19:01:08Z

cmd/internal/kube/daemonset/kube_events_worker.go

+	// Empty podArgs object for PodPending state
+	args := &PodArgs{
+		PodName: pod.Name,
+		// though 1 buffer size is enough, keeping 2 for safety
+		StopChan: make(chan error, 2),
 	}


This creation of a PodArgs in a particular state should be encapsulated in a function? It appears at least twice.

mgritter · 2025-02-19T19:02:04Z

cmd/internal/kube/daemonset/kube_events_worker.go

 	}
+	printer.Debugf("Pod in pending state added to map, pod name: %s\n", pod.Name)


The log message makes it sound like it has already occurred, but we could fail -- maybe move this below the error check, or adjust its wording?

mgritter · 2025-02-19T19:06:03Z

data_masks/redactor.go

+
+	// Whether this redactor is still running. False if and only if exitChannel is
+	// closed.
+	running *atomic.Bool


Sorry, any use of an atomic.Bool is a code smell, particularly if you're trying to signal the state of some other object. Is there a higher-level construct that will do the job?

It looks like what you want is for StopPeriodicUpdates() to call close() exactly once. Maybe a sync.Once is the appropriate thing then?

This commit is from another PR for POA-2904 which is already merged to master.
I had raised the initial PR #81, and then Jed raised his own PR #83, which we merged.

We can discuss this in the ticket if there should be a change in implementation.

I'm not sure why those changes were merged into this branch, then?

Fixes from other PRs should go into the main branch or a feature branch, then merged from there back into the development branch, so they don't have to be reviewed twice.

…hts-agent into mudit/event-watcher-changes

Child PRs: #67, #69, #71, #72, #73, #74, #75, #76, #77, #78, #79, #80, #82

mudit-postman added 18 commits February 14, 2025 15:36

Chores:

49306e9

- Add logs before each process start - Move default interval time to daemonset module:

chore: change StateChangeMutex to sync.Mutex instead to pointer

38dcb25

chore: change PodTrafficMonitorState from int to string

317c5e1

chore: pass pointer of mutext parent and run go mod tidy

79d2b4e

chore: use for range instead of single switch case

4fe1937

chore: set revisionVersion to only fetch events after agent creation

2b63162

fix: parent run command starting in separate go routine

0335c52

Fixes:

b8f9005

- Handle case if any state change is allowed in changePodTrafficMonitorState() - if condition in addPodArgsToMap() is opposite also not this func will return error

chore: if pod is already running but agent is not listening to it sta…

aa932f6

…rt apidump process in next healthcheck run

fix: prepend '/host' to network namespace

d11593e

fix: pass learnClient in util.NewLearnSession()

7718c43

chore: set default apidump args

2168fc4

fix:

3811e45

- telemetry api erroring our due to nil resp var - already deleted pod is not getting removed during pod health check

fix:

295b355

- telemetry api erroring our due to nil resp var - already deleted pod is not getting removed during pod health check

Merge branch 'mudit/poa-2609-testing-changes' of github.com:postmanla…

a9075e0

…bs/postman-insights-agent into mudit/poa-2609-testing-changes

chores:

2e4cde2

- Change from event watcher to pod watcher in k8s - Changes in k8s event worker, also added modified state handler - Remove state change to traffic stopped from traffic ended and errored - Change inspectPodForEnvVars func, it will now except podArgs object

Chores:

6e5f658

- Added sync.WaitGroup for apidump processes - Refactored states to make them same as k8s pods states

chore: improve pods map dump formatting

2290089

mudit-postman requested a review from mgritter February 18, 2025 14:03

mudit-postman marked this pull request as ready for review February 18, 2025 14:03

mudit-postman changed the title ~~Changed the event watcher to watch pod changes not event changes~~ [POA-2928] Changed the event watcher to watch pod changes not event changes Feb 18, 2025

liujed and others added 6 commits February 18, 2025 22:52

Merge branch 'main' of github.com:postmanlabs/postman-insights-agent …

c8b4193

…into mudit/event-watcher-changes

chore: refactor ENV vars

2a7667c

chore: refactor ENV vars 2

15eb4bc

chore: fix failing tests

0fb81ee

chore: merge POSTMAN_ENV and POSTMAN_INSIGHTS_ENV

33e5b2a

mgritter approved these changes Feb 19, 2025

View reviewed changes

Boomtokn approved these changes Feb 19, 2025

View reviewed changes

Base automatically changed from mudit/poa-2609-testing-changes to mudit/poa-2609 February 19, 2025 20:23

mudit-postman added 2 commits February 20, 2025 02:07

Merge branch 'mudit/poa-2609' of github.com:postmanlabs/postman-insig…

3dcabf3

…hts-agent into mudit/event-watcher-changes

chore: address PR comments

4f12d50

mudit-postman merged commit de58c5e into mudit/poa-2609 Feb 19, 2025
2 checks passed

mudit-postman deleted the mudit/event-watcher-changes branch February 19, 2025 20:47

mudit-postman added a commit that referenced this pull request Feb 19, 2025

[POA-2609] Add k8s and CRI sub-modules and function interfaces (#66)

6027135

Child PRs: #67, #69, #71, #72, #73, #74, #75, #76, #77, #78, #79, #80, #82

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[POA-2928] Changed the event watcher to watch pod changes not event changes #82

[POA-2928] Changed the event watcher to watch pod changes not event changes #82

mudit-postman commented Feb 18, 2025 •

edited

Loading

mgritter left a comment •

edited

Loading

mgritter Feb 19, 2025

mgritter Feb 19, 2025

mgritter Feb 19, 2025

mudit-postman Feb 19, 2025

mgritter Feb 19, 2025

		}
		printer.Debugf("Pod in pending state added to map, pod name: %s\n", pod.Name)

[POA-2928] Changed the event watcher to watch pod changes not event changes #82

[POA-2928] Changed the event watcher to watch pod changes not event changes #82

Conversation

mudit-postman commented Feb 18, 2025 • edited Loading

mgritter left a comment • edited Loading

Choose a reason for hiding this comment

mgritter Feb 19, 2025

Choose a reason for hiding this comment

mgritter Feb 19, 2025

Choose a reason for hiding this comment

mgritter Feb 19, 2025

Choose a reason for hiding this comment

mudit-postman Feb 19, 2025

Choose a reason for hiding this comment

mgritter Feb 19, 2025

Choose a reason for hiding this comment

mudit-postman commented Feb 18, 2025 •

edited

Loading

mgritter left a comment •

edited

Loading