fix: send batch-submitter error logs to sentry#525
Conversation
🦋 Changeset detectedLatest commit: 7836892 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GCP will read in logs via
Looking at the comments following the link you posted, I think they have really high standards for fast (which is good) and we do enough network i/o to where that is likely the bottleneck and some additional overhead when logging won't be that big of a deal. We will still want logs going to |
snario
left a comment
There was a problem hiding this comment.
We should not disable STDOUT.
| tracesSampleRate: 0.05, | ||
| level: 'error' | ||
| const log = new Logger({ | ||
| name: 'oe:batch-submitter:init', |
There was a problem hiding this comment.
Is this conforming to some known format?
There was a problem hiding this comment.
i have no idea... @karlfloersch might have more context here
snario
left a comment
There was a problem hiding this comment.
Conditional approval on fixing the suggested change (i.e., .concat does not mutate)
ad42d07 to
17e441e
Compare
* fix: send batch-submitter error logs to sentry * chore: changeset * move sentry to logger and use multi-stream
<!-- Thank you for your Pull Request. Please provide a description above and review the requirements below. Bug fixes and new features should include tests. Contributors guide: https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md The contributors guide includes instructions for running rustfmt and building the documentation. --> <!-- ** Please select "Allow edits from maintainers" in the PR Options ** --> ## Motivation Ref alloy-rs/alloy#2478 `OpTransactionRequest` is a wrapper around `TransactionRequest`. It doesn't implement `TransactionBuilder4844` (It shouldn't). But this makes it incompatible with `ProviderBuilder` default constructor `new` <!-- Explain the context and why you're making that change. What is the problem you're trying to solve? In some cases there is not a problem and this can be thought of as being the motivation for your change. --> ## Solution - Remove OpTransactionRequest - Move related `From` conversions - Use TransactionRequest in network impl <!-- Summarize the solution and provide any necessary context needed to understand the code change. --> ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes --------- Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
…ionRequest` of `Optimism` (#557) ## Motivation The change #525 introduced ambiguity of `alloy_network::TransactionBuilder` implementation for `TransactionRequest`, being implemented twice once for `Ethereum` and once for `Optimism`. This then requires fully qualified syntax to differentiate. ## Solution Bring back `OpTransactionRequest` ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
…ionRequest` of `Optimism` (alloy-rs/op-alloy#557) ## Motivation The change #525 introduced ambiguity of `alloy_network::TransactionBuilder` implementation for `TransactionRequest`, being implemented twice once for `Ethereum` and once for `Optimism`. This then requires fully qualified syntax to differentiate. ## Solution Bring back `OpTransactionRequest` ## PR Checklist - [ ] Added Tests - [ ] Added Documentation - [ ] Breaking changes
Description
Batch submitter errors weren't being properly sent to Sentry before, due to the way the destination was initialized. This PR adds multi-stream functionality to
Logger, and has been tested and sendslog.errors in the Batch Submitter to Sentry.Additional context
This now writes to both Sentry and
stdout!