Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions error_reporting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,6 @@

This section contains samples for [Google Cloud Error Reporting](https://cloud.google.com/error-reporting).

A startup script has been provided to demonstrated how to properly provision a GCE
instance with fluentd configured. Note the intallation of fluentd, the addition of the config file,
and the restarting of the fluetnd service. You can start an instance using
it like this:

gcloud compute instances create example-instance --metadata-from-file startup-script=startup_script.sh

or simply use it as reference when creating your own instance.

After fluentd is configured, main.py could be used to simulate an error:

gcloud compute copy-files main.py example-instance:~/main.py

Then,

gcloud compute ssh example-instance
python ~/main.py

And you will see the message in the Errors Console.

<!-- auto-doc-link -->
These samples are used on the following documentation page:
Expand Down
18 changes: 3 additions & 15 deletions error_reporting/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,16 @@
# limitations under the License.

# [START error_reporting]
import traceback

import fluent.event
import fluent.sender
from google.cloud import error_reporting


def simulate_error():
fluent.sender.setup('myapp', host='localhost', port=24224)

def report(ex):
data = {}
data['message'] = '{0}'.format(ex)
data['serviceContext'] = {'service': 'myapp'}
# ... add more metadata
fluent.event.Event('errors', data)

# report exception data using:
client = error_reporting.Client()
try:
# simulate calling a method that's not defined
raise NameError
except Exception:
report(traceback.format_exc())
client.report_exception()
# [END error_reporting]


Expand Down
8 changes: 5 additions & 3 deletions error_reporting/main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
import main


@mock.patch("fluent.event")
def test_error_sends(event_mock):
@mock.patch("main.error_reporting")
def test_error_sends(error_reporting_mock):
client_mock = mock.Mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a system test, it's a unit test. Why not forgo mocking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure exactly what you mean? Without mocks there's not much to assert, seemed better to throw in one mock to assert something.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have several tests that don't assert, they implicitly assert the absence of failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well with the mock, it verifies it took the code path we actually expected, and doesn't add an actual network round trip to our unit test. Testing the report_exception call itself belongs in the client library. So seems to me to be slightly better to keep the mock.

Copy link
Contributor

Choose a reason for hiding this comment

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

with the mock we actually test nothing here. The client library can change and break the sample and tests would never fail, which is why we don't write unit tests here. Everything in this repo is a system test so I do not care about the cost of making an RPC.

If you want to assert that the call is being made, use wraps.

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 doesn't test nothing, it tests the code path, which it doesn't without the mock. wraps seems like it cleans up the mocking code. However, can see both sides and not worth arguing about so made change.

error_reporting_mock.Client.return_value = client_mock
main.simulate_error()
event_mock.Event.assert_called_once_with(mock.ANY, mock.ANY)
client_mock.report_exception.assert_called_once()
2 changes: 1 addition & 1 deletion error_reporting/requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
fluent-logger==0.4.4
google-cloud-error-reporting==0.21.0
35 changes: 0 additions & 35 deletions error_reporting/startup_script.sh

This file was deleted.