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: add event metadata as json to publish error logs #188

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

rgraber
Copy link
Contributor

@rgraber rgraber commented Aug 8, 2023

Add metadata in json form to the error logs when events fail to publish. This will be useful for republishing the events later.
Issue:
edx/edx-arch-experiments#354

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed
  • Noted any: Concerns, dependencies, deadlines, tickets, testing instructions

@@ -247,6 +247,7 @@ def test_full_event_data_present_in_key_extraction_error(self, mock_logger, *arg
assert f"sourcehost='{metadata.sourcehost}'" in error_string
assert "event_data_as_json='{\"test_data\": {\"course_id\": \"id\", \"sub_name\": \"name\"}}'"\
in error_string
assert f"event_metadata_as_json='{metadata.to_json()}'" in error_string
Copy link
Contributor

Choose a reason for hiding this comment

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

[observation] It seems some of the metadata is now duplicated. I guess that's not a big deal if this is just for errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is indeed duplicated. I thought we'd be able to use the columns from Splunk to make a metadata object, but the problem is there's a collision in the "source" field, which Splunk uses to store other data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose on further reflection we could have just changed how the source shows up in the string representation but that would mean making a change specifically for Splunk!reasons in openedx-events, whereas the json representation seems like a more general solution. I don't think it's too much of a load on the logs to add this, especially because we only log on failure.

@rgraber rgraber merged commit 860ec85 into main Aug 9, 2023
8 checks passed
@rgraber rgraber deleted the rsgraber/20230808-add-event-metadata-json-to-logs branch August 9, 2023 12:09
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