Added a timer goroutine to run for a specific duration and upon expir… #1519

heyyakash · 2024-02-07T14:54:20Z

…ation sends an os.Interrupt signal which shuts down the recorder and exits the program

Related Issue

Add a timer to the keploy recorder
references closed pr [Feature] : Added a timer to keploy recorder #1484 (comment)

Closes: #1434

Describe the changes you've made

Added a timer (-t) flag (eg. -t 50s) to record cmd which takes the input for the time for which the user wants the recorder to run and defaults to 0 if none is provided.
Added a timer goroutine in captureTraffic() in record.go file that initiates the timer just before user application is launched and once the timer is up sends os.Interrupt signal to stopper channel to gracefully stop the recorder.
If the value of timer is 0 (default value) the timer won't run and the recorder can run as long as user wants.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, local variables)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Please let us know if any test cases are added

Please describe the tests(if any). Provide instructions how its affecting the coverage.

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

A clear and concise description of it.
NA

Checklist:

My code follows the style guidelines of this project.
I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.

Screenshots (if any)

Original	Updated

…ation sends an os.Interrupt signal which shuts down the recorder and exits the program Signed-off-by: heyyakash <akashsharma2002@gmail.com>

heyyakash · 2024-02-07T14:57:43Z

@PranshuSrivastava had to force push to match my local branch to original branch and that led to closing of previous pr. Can you review this one instead

PranshuSrivastava · 2024-02-07T16:03:59Z

cmd/record.go

@@ -42,7 +42,7 @@ func readRecordConfig(configPath string) (*models.Record, error) {

 var filters = models.TestFilter{}

-func (t *Record) GetRecordConfig(path *string, proxyPort *uint32, appCmd *string, appContainer, networkName *string, Delay *uint64, buildDelay *time.Duration, passThroughPorts *[]uint, passThrough *[]models.Filters, configPath string) error {
+func (t *Record) GetRecordConfig(path *string, proxyPort *uint32, appCmd *string, appContainer, networkName *string, Delay *uint64, buildDelay *time.Duration, passThroughPorts *[]uint, passThrough *[]models.Filters, configPath string, keployRecorderStopTimer *time.Duration) error {


This name is too long. Just keep it something simple like "recordTimer"

PranshuSrivastava · 2024-02-07T16:05:53Z

cmd/record.go

@@ -303,6 +308,8 @@ func (r *Record) GetCmd() *cobra.Command {

 	recordCmd.Flags().StringP("command", "c", "", "Command to start the user application")

+	recordCmd.Flags().DurationP("keployRecorderStopTimer", "t", 0, "Command to initiate a timer that automatically stops the Keploy recorder after a specified duration of time")


Make this also into something briefer like "timer to stop keploy record after the required time".

Signed-off-by: heyyakash <akashsharma2002@gmail.com>

heyyakash · 2024-02-07T19:05:34Z

@PranshuSrivastava I've made the changes, please check

PranshuSrivastava

I've tested your changes, they work as intended, just make the following changes.

PranshuSrivastava · 2024-02-08T17:59:28Z

pkg/service/record/record.go

+				timer := time.After(recordTimer)
+				select {
+				case <-timer:
+					r.Logger.Warn("Time up! Stopping the timer")


we are stopping keploy not the timer.

PranshuSrivastava · 2024-02-08T18:07:20Z

cmd/record.go

@@ -303,6 +308,8 @@ func (r *Record) GetCmd() *cobra.Command {

 	recordCmd.Flags().StringP("command", "c", "", "Command to start the user application")

+	recordCmd.Flags().DurationP("recordTimer", "t", 0, "Timer to stop keploy recorder after a specified time")


why is the shorthand of the flag -t? That can confuse the user, either start the name with t or change the shorthand to the first alphabet of the full flag name.

Heyy! so i removed the shorthand entirely and kept it as --recordTimer , keeping the shorthand as -r would lead to ambiguity, So please check

Signed-off-by: heyyakash <akashsharma2002@gmail.com>

heyyakash · 2024-02-10T15:37:08Z

@PranshuSrivastava can you please review the new changes?

PranshuSrivastava · 2024-02-11T17:17:18Z

cmd/record.go

@@ -308,7 +308,7 @@ func (r *Record) GetCmd() *cobra.Command {

 	recordCmd.Flags().StringP("command", "c", "", "Command to start the user application")

-	recordCmd.Flags().DurationP("recordTimer", "t", 0, "Timer to stop keploy recorder after a specified time")
+	recordCmd.Flags().Duration("recordTimer", 0, "Timer to stop keploy recorder after a specified time")


Why have you removed the code for reading this flag?

heyy! so i removed the shorthand entirely and kept it as --recordTimer , keeping the shorthand as -r would lead to ambiguity,

PranshuSrivastava

LGTM

heyyakash · 2024-02-15T06:56:59Z

@PranshuSrivastava can you merge then?

Added a timer goroutine to run for a specific duration and upon expir…

be15d5a

…ation sends an os.Interrupt signal which shuts down the recorder and exits the program Signed-off-by: heyyakash <akashsharma2002@gmail.com>

PranshuSrivastava requested changes Feb 7, 2024

View reviewed changes

heyyakash and others added 2 commits February 8, 2024 00:24

Merge branch 'keploy:main' into timer

5859fd2

Changed the description and the variable name of timer command

ea9d80d

Signed-off-by: heyyakash <akashsharma2002@gmail.com>

Merge branch 'main' into timer

5c4f052

PranshuSrivastava reviewed Feb 8, 2024

View reviewed changes

Updated shorthand and stopping message

758c5d7

Signed-off-by: heyyakash <akashsharma2002@gmail.com>

PranshuSrivastava requested changes Feb 11, 2024

View reviewed changes

Merge branch 'main' into timer

756631f

PranshuSrivastava approved these changes Feb 13, 2024

View reviewed changes

heyyakash added 2 commits February 14, 2024 00:09

Merge branch 'main' into timer

85139f3

Merge branch 'main' into timer

abdcea1

Merge branch 'main' into timer

8bfffb8

PranshuSrivastava merged commit 80d9f7f into keploy:main Feb 15, 2024
13 checks passed

github-actions bot locked and limited conversation to collaborators Feb 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a timer goroutine to run for a specific duration and upon expir… #1519

Added a timer goroutine to run for a specific duration and upon expir… #1519

heyyakash commented Feb 7, 2024

heyyakash commented Feb 7, 2024

PranshuSrivastava Feb 7, 2024

heyyakash Feb 7, 2024

PranshuSrivastava Feb 7, 2024

heyyakash commented Feb 7, 2024

PranshuSrivastava left a comment

PranshuSrivastava Feb 8, 2024

PranshuSrivastava Feb 8, 2024

heyyakash Feb 9, 2024

heyyakash commented Feb 10, 2024

PranshuSrivastava Feb 11, 2024

heyyakash Feb 11, 2024 •

edited

Loading

PranshuSrivastava left a comment

heyyakash commented Feb 15, 2024

		@@ -303,6 +308,8 @@ func (r Record) GetCmd() cobra.Command {

		recordCmd.Flags().StringP("command", "c", "", "Command to start the user application")

		recordCmd.Flags().DurationP("keployRecorderStopTimer", "t", 0, "Command to initiate a timer that automatically stops the Keploy recorder after a specified duration of time")

		@@ -303,6 +308,8 @@ func (r Record) GetCmd() cobra.Command {

		recordCmd.Flags().StringP("command", "c", "", "Command to start the user application")

		recordCmd.Flags().DurationP("recordTimer", "t", 0, "Timer to stop keploy recorder after a specified time")

Added a timer goroutine to run for a specific duration and upon expir… #1519

Added a timer goroutine to run for a specific duration and upon expir… #1519

Conversation

heyyakash commented Feb 7, 2024

Related Issue

Describe the changes you've made

Type of change

Please let us know if any test cases are added

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

Checklist:

Screenshots (if any)

heyyakash commented Feb 7, 2024

PranshuSrivastava Feb 7, 2024

Choose a reason for hiding this comment

heyyakash Feb 7, 2024

Choose a reason for hiding this comment

PranshuSrivastava Feb 7, 2024

Choose a reason for hiding this comment

heyyakash commented Feb 7, 2024

PranshuSrivastava left a comment

Choose a reason for hiding this comment

PranshuSrivastava Feb 8, 2024

Choose a reason for hiding this comment

PranshuSrivastava Feb 8, 2024

Choose a reason for hiding this comment

heyyakash Feb 9, 2024

Choose a reason for hiding this comment

heyyakash commented Feb 10, 2024

PranshuSrivastava Feb 11, 2024

Choose a reason for hiding this comment

heyyakash Feb 11, 2024 • edited Loading

Choose a reason for hiding this comment

PranshuSrivastava left a comment

Choose a reason for hiding this comment

heyyakash commented Feb 15, 2024

Describe if there is any unusual behaviour of your code(Write `NA` if there isn't)

heyyakash Feb 11, 2024 •

edited

Loading