Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: log session action completion events #49

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

jusiskin
Copy link
Contributor

@jusiskin jusiskin commented Oct 6, 2023

What was the problem/requirement? (What/Why)

Session action completion events were not being logged

What was the solution? (How)

Added logic for logging session action completion events.

What is the impact of this change?

All session and session action lifecycle events are now emitted

How was this change tested?

  • Automated unit tests were added
  • Manual end-to-end testing

Was this change documented?

No

Is this a breaking change?

No

@jusiskin jusiskin added the enhancement New feature or request label Oct 6, 2023
@jusiskin jusiskin requested a review from a team as a code owner October 6, 2023 05:35
Copy link
Contributor

@ddneilson ddneilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid improvement, Josh. However, this -- environment[STEP:step-e404e034a52846809db3ec250ea82f48:myenv].enter() -- sort of message in the log has been identified as being the opposite of human readable; there's a ticket that calls it out explicitly. It's readable to programmer-minded individuals, but little else.

Do you have thoughts on how that can also be improved?

One idea: environment[STEP:step-e404e034a52846809db3ec250ea82f48:myenv].enter() -> "[<stepid>] Entering Environment: myenv". Ideal might have been "Entering Step/Job/External Environment...", but the Worker doesn't have that context.

@ddneilson
Copy link
Contributor

Solid improvement, Josh. However, this -- environment[STEP:step-e404e034a52846809db3ec250ea82f48:myenv].enter() -- sort of message in the log has been identified as being the opposite of human readable; there's a ticket that calls it out explicitly. It's readable to programmer-minded individuals, but little else.

Do you have thoughts on how that can also be improved?

One idea: environment[STEP:step-e404e034a52846809db3ec250ea82f48:myenv].enter() -> "[<stepid>] Entering Environment: myenv". Ideal might have been "Entering Step/Job/External Environment...", but the Worker doesn't have that context.

Let's take this improvement as a followup.

@jusiskin jusiskin merged commit bb4e3bd into mainline Oct 6, 2023
8 checks passed
@jusiskin jusiskin deleted the jusiskin/log_completed_session_actions branch October 6, 2023 20:07
jusiskin added a commit that referenced this pull request Oct 6, 2023
@jusiskin jusiskin changed the title feat!: session lifecycle log improvements feat: log session action completion events Oct 6, 2023
gmchale79 pushed a commit that referenced this pull request Oct 31, 2023
Signed-off-by: Josh Usiskin <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gmchale79 pushed a commit that referenced this pull request Nov 2, 2023
Signed-off-by: Josh Usiskin <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gahyusuh pushed a commit that referenced this pull request Nov 6, 2023
Signed-off-by: Josh Usiskin <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
Signed-off-by: Gahyun Suh <[email protected]>
gmchale79 pushed a commit that referenced this pull request Feb 12, 2024
Signed-off-by: Josh Usiskin <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gmchale79 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Josh Usiskin <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
jusiskin added a commit to jusiskin/deadline-cloud-worker-agent that referenced this pull request Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants