refactor: replaced dbg! with trace macro

[taddes]

Description

Simple change to replace dbg! marco with the slog-rs trace! macro, since the dbg! was resulting in stderr output, not to stdout.

Testing

Compiles and matches patterns in rest of handlers.rs file.
Had to make alterations within the .circleci yaml file. @pjenvey was essential in identifying issue where cargo build pulls in most recent rust version which is often at odds with the specified version in the Docker build, which is explicitly hard-coded.
Commented out commands to build rust and update which would override specified version in CI/CD. In the future, these lines can be commented back in and a | inserted at start of command directive.
Also, it was necessary to add the no_output_timeout directive to cargo build to allow for longer compilation times.

How should reviewers test?
Ensure GCP logs show output of trace!("Batch: Appending to {}", &new_batch.id); replacing previous dbg! here

Issue(s)

Closes link.

[taddes]

@pjenvey hoping to discuss this and get some additional context on slog-rs and if there is a better pattern. Also need to ensure I can verify the output from slog in GCP.

[ethowitz]

Were these commands commented out just because there was a temporary mismatch between the Rust version specified in the Dockerfile and the Rust version being pulled in here? If so, we can probably the Dockerfile and uncomment these lines now, right? Looks like there's a 1.61 buster image now: https://hub.docker.com/_/rust

[ethowitz]

Alternatively, do we need these commands at all if we always want to use the Rust version given to us by the Docker image?

[jrconlin]

FWIW, Rust Docker images usually are available the day after a rust release. It's been quite some time since there's been a breaking change between versions of rust, or a rust update causing something like fmt or clippy to fail. I'd be curious why those were commented out.

[taddes]

I talked with Phil and he had said this had happened in previous builds. I did a commenting out of the steps here so as to not eliminate them until we'd discussed the need or lack of need to have those commands there. There were no local compilation issues on my machine and it was the CircleCI builds breaking due to this reason and we found some evidence of other cases.

[jrconlin]

Isn't cargo build fairly chatty?

20m of no output seems a bit long. Why are we using this value?

[taddes]

I discussed with Phil and we thought that this helps avoid the problem of there not being chatty output, which in this case was the problem. We saw stalling on the MySQL side of things and if you look, it took 22 min to cargo-build, 32 for the docker image. If you can think of another way to handle this, it sounds good to me, but this ended up fixing the problem when it came to passing the CircleCI checks.

[ethowitz]

It might be worth investigating ways to cut down on the syncstorage build time as part of the sync modernization work because it is getting quite lengthy

[ethowitz]

FWIW, it appears the source of this may actually be a regression in Rust 1.61.0 itself. I've opened a PR (#1326) to address this by pinning Rust to 1.60.0. As @pjenvey noted here, we should probably start pinning to a particular Rust version anyway (instead of just grabbing the latest version) to ensure that the version in the Dockerfile and the version in the CircleCI file always match.

[taddes]

Agreed @ethowitz! Since that PR is now merged, how would you like to proceed with closing this out given the changes in the CircleCI config? Thanks!

[ethowitz]

You can go ahead and rebase with the master branch to pull the changes in. I think we can remove this line (the one that extends the timeout to 20 minutes). From what I can tell, I think we should also remove the rustup commands you commented out: we're using circleci images that have the correct version of rust pre-installed, so I think those commands prob aren't necessary

[taddes]

Ok, I made the changes to the file, only leaving the output of what Rust version is currently built, though is there any merit to leaving the 20 min timeout? I'm letting it run now to see how long it may take to build again given these changes.

[ethowitz]

This looks good to me!