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

Log to GitLab when the job handler cannot be created #6

Closed
wants to merge 1 commit into from

Conversation

refi64
Copy link
Collaborator

@refi64 refi64 commented Oct 21, 2022

This can happen due to e.g. configuration issues on the repo, so it makes sense to log it somewhere other than just the service logs.

Signed-off-by: Ryan Gonzalez [email protected]

This can happen due to e.g. configuration issues on the repo, so it
makes sense to log it somewhere other than just the service logs.

Signed-off-by: Ryan Gonzalez <[email protected]>
@@ -140,7 +140,7 @@ async fn main() {
|job| async {
ObsJobHandler::from_obs_config_in_job(job, HandlerOptions::default()).map_err(
|err| {
error!("Failed to create new client: {:?}", err);
error!(gitlab.output = true, "Failed to handle new job: {:?}", err);
Copy link
Contributor

Choose a reason for hiding this comment

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

gitlab.output is really an implementation detail that crates using the runner shouldn't be aware of :). If you use outputln! it will already do the right thing; If you want and equivalent of e.g. error! then that's something to look at in the gitlab runner crate really.

refi64 added a commit that referenced this pull request Nov 2, 2022
While thinking on #6 and the accidental / intentional use of
gitlab.output, it occurred to me that the way we show errors for GitLab
is not actually how we'd want them logged with JSON logging (which
currently dumps the entire error string, *including ANSI codes*, inside
the JSON objects). With this in mind, I realized we can get the
color-eyre handler's span traces and just convert it to JSON manually.

This would be all, if we could actually log JSON from tracing.
Unfortunately, any extra fields logged are converted to strings via
Debug and stored as such in the JSON fields. That means we end up with
our beautiful JSON errors stuck as strings and filled with dozens of
escape sequences, which also makes them entirely unreadable without
taking the JSON apart.

But! tracing has "unstable" (can change in a bugfix release, but isn't
that what Cargo.lock is for anyway?) functionality that integrates with
the valuable crate, such that any field value that's a Valuable will be
serialized as JSON. So, the eyre Report is converted to an intermediate
Valuable that gets sent to tracing, which dutifully serializes it as an
actual JSON object.

This does, however, still have an unfortunate missing piece, which is
serializing the fields in the span trace. tracing-error serializes
fields into a string to be retrieved when the span trace is collected,
which isn't the JSON format we would want. Now, this *can* be changed to
format JSON, but that's not very useful because:

- It means that span trace fields will be shown as JSON in the GitLab
  logs too, which isn't great.
- There is no straightforward way to go from a JSON string to a
  Valuable: valuable only includes ways to go serialize a Valuable via
  Serde, without any way of *deserializing*. This would only be possible
  by manually converting everything, which is increasing the
  already-high complexity.

In practice, the individual field values don't really have complex JSON
values, so leaving the default non-JSON formatting there is fine.

Signed-off-by: Ryan Gonzalez <[email protected]>
@refi64
Copy link
Collaborator Author

refi64 commented Nov 2, 2022

Superseded by #9.

@refi64 refi64 closed this Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants