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

Emit telemetry events on success and failure of the Create Job, Create Job Definition, Create Job from Job Definition hooks #472

Merged
merged 9 commits into from
Jan 22, 2024

Conversation

andrii-i
Copy link
Collaborator

@andrii-i andrii-i commented Jan 12, 2024

Emit telemetry events on success and failure of the Create Job, Create Job Definition, Create Job from Job Definition hooks.

  • Log object's body.name string is now following a new convention: org.jupyter.jupyter-scheduler.SOURCE.EVENT[.STATUS] (vs old convention org.jupyter.jupyter-scheduler.SOURCE.EVENT) where STATUS is either "success" or "failure"
  • CreateJob now emits "create-job" or "create-job-definition" for EVENT instead of just "create" depending on if job or job definition is being created
  • Log object's body now has optional detail parameter storing error message (or other string with event detail)
type EventLog = {
  body: { name: string; detail?: string };
  timestamp: Date;
};
  • Improve error catching logic by catching errors as unknown type and using type guarding instead of assuming all errors caught would be of Error type (ts runtime error type) which is not guaranteed.
  • Fix Emit telemetry events on create job forms #471

Emitted objects examples:
Screenshot 2024-01-18 at 2 52 01 PM

Screenshot 2024-01-18 at 2 50 31 PM

@andrii-i andrii-i added the enhancement New feature or request label Jan 12, 2024
@dlqqq dlqqq requested a review from 3coins January 16, 2024 19:17
@dlqqq dlqqq marked this pull request as draft January 16, 2024 19:18
@andrii-i andrii-i marked this pull request as ready for review January 17, 2024 23:10
@andrii-i andrii-i requested a review from bhadrip January 17, 2024 23:10
@andrii-i andrii-i marked this pull request as draft January 18, 2024 19:12
@andrii-i andrii-i requested a review from 3coins January 18, 2024 22:13
@andrii-i andrii-i marked this pull request as ready for review January 18, 2024 22:13
@andrii-i
Copy link
Collaborator Author

@3coins thank you for looking into this, implemented your comments re not logging an httpResponseCode. Also added improved error catching logic as discussed.

Copy link
Collaborator

@JasonWeill JasonWeill left a comment

Choose a reason for hiding this comment

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

"Unknown error occurred" is a generic message, but it may not inspire confidence. Do we have any idea about what sorts of errors might trigger the generic message? If so, can we provide any additional guidance? Something like, "An error occurred. Please try again."?

@andrii-i
Copy link
Collaborator Author

@JasonWeill thank you for looking into this. Any error / error object that does not have fields expected from es5 Error object would trigger the generic error message. Most probably to occur are connection failures, connection timeouts, third-party libraries throwing something else than es5 Error (other objects or just strings). So the message needs to be generic, I think the message that you proposed works well, updated the function to use it.

@andrii-i andrii-i requested a review from JasonWeill January 19, 2024 22:30
Copy link
Collaborator

@JasonWeill JasonWeill left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@andrii-i andrii-i merged commit eeb5de2 into jupyter-server:main Jan 22, 2024
6 checks passed
@andrii-i andrii-i deleted the telemetry_creation_success branch January 22, 2024 18:56
andrii-i added a commit that referenced this pull request Jan 23, 2024
…e Job Definition, Create Job from Job Definition hooks (#472)

* add job/job definition creation event success or failure logging

* fix create-job logging format

* log error message and httpStatusCode when error occurs

* Update src/index.tsx

Co-authored-by: Piyush Jain <[email protected]>

* Update src/context.ts

Co-authored-by: Piyush Jain <[email protected]>

* Update src/context.ts

Co-authored-by: Piyush Jain <[email protected]>

* log string for logDetails, not whole error

* catch errors as unknown, not Error

* Make generic error message more user-frinedly and actionable per @JasonWeill

---------

Co-authored-by: Piyush Jain <[email protected]>
andrii-i added a commit to andrii-i/jupyter-scheduler that referenced this pull request Jan 23, 2024
…e Job Definition, Create Job from Job Definition hooks (jupyter-server#472)

* add job/job definition creation event success or failure logging

* fix create-job logging format

* log error message and httpStatusCode when error occurs

* Update src/index.tsx

Co-authored-by: Piyush Jain <[email protected]>

* Update src/context.ts

Co-authored-by: Piyush Jain <[email protected]>

* Update src/context.ts

Co-authored-by: Piyush Jain <[email protected]>

* log string for logDetails, not whole error

* catch errors as unknown, not Error

* Make generic error message more user-frinedly and actionable per @JasonWeill

---------

Co-authored-by: Piyush Jain <[email protected]>
andrii-i added a commit that referenced this pull request Jan 23, 2024
…e Job Definition, Create Job from Job Definition hooks (#472) (#475)

* add job/job definition creation event success or failure logging

* fix create-job logging format

* log error message and httpStatusCode when error occurs

* Update src/index.tsx



* Update src/context.ts



* Update src/context.ts



* log string for logDetails, not whole error

* catch errors as unknown, not Error

* Make generic error message more user-frinedly and actionable per @JasonWeill

---------

Co-authored-by: Piyush Jain <[email protected]>
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.

Emit telemetry events on create job forms
4 participants