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

Document upcoming changes to webhooks payloads #1282

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fkorotkov
Copy link
Contributor

These are proposed new fields that will come to the payloads and GraphQL API in the coming weeks.

These events will allow to improve export of the events into systems like Datadog. See the following issue:

These are proposed new fields that will come to the payloads and GraphQL API in the coming weeks.

These events will allow to improve export of the events into systems like Datadog. See the following issue:

* cirruslabs/cirrus-webhooks-server#8
* cirruslabs/cirrus-webhooks-server#9
@fkorotkov fkorotkov requested a review from edigaryev July 16, 2024 02:53
Copy link
Contributor

@edigaryev edigaryev left a comment

Choose a reason for hiding this comment

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

This needs a timestamp field for build's and task's, similarly to audit_event's.

Currently, for build's we only emit a single timestamp, which is build.changeTimestamp, but it doesn't change when the build gets updated.

For tasks we could use the statusTimestamp, but it would be nice to have a uniform timestamp field to avoid ambiguity for the webhook receiver implementors.

@fkorotkov
Copy link
Contributor Author

@edigaryev what do you think about a request header. Since the payload is a GraphQL query response and I don't think we can add timestamp there. 🤔

@edigaryev
Copy link
Contributor

@edigaryev what do you think about a request header. Since the payload is a GraphQL query response and I don't think we can add timestamp there. 🤔

That would totally work too.

@edigaryev
Copy link
Contributor

Since the payload is a GraphQL query response and I don't think we can add timestamp there. 🤔

On a side note, it seems that the action and old_status fields are merged somehow, so probably we can:

Screenshot 2024-07-22 at 15 49 20

@edigaryev
Copy link
Contributor

Also, perhaps we should make the audit_event to conform to this format, or vice versa, because currently it differs, see screenshot below.

Note that the body is not a nested object, but instead a JSON string:

Screenshot 2024-07-22 at 15 57 21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants