Submit diagnose reports to spec server #23
Conversation
Rename the expect_section to expect_output_for. This name is more clear, it asserts the output of the runner, for a specific section.
tombruijn
force-pushed
the
transmitted-report
branch
3 times, most recently
from
828430f to
8ad3e89
Compare
tombruijn
force-pushed
the
transmitted-report
branch
3 times, most recently
from
4236732 to
4e48da5
Compare
| end | ||
|
|
||
| it "submit a report automatically" do | ||
| expect(@received_report).to be_instance_of(DiagnoseReport) |
There was a problem hiding this comment.
It would be good to check somewhere (not sure if here's the right place) that the api_key param is sent along when a report is submitted, to cover the fix for its absence at appsignal/appsignal-nodejs#474.
Additionally, it could be good to check that only one report is received, more as a "sanity check" than anything.
There was a problem hiding this comment.
Yes, that would be a good thing to test as well later.
| end | ||
| end | ||
|
|
||
| set :logging, false |
There was a problem hiding this comment.
The internet says that adding set :quiet, false will get rid of some of the extra logging messages on standard output (the cutesy ones regarding entering and exiting stages, I think)
Also, since this is a subclass of Sinatra::Base, the docs say (ctrl-f ":logging") that it should already have set :logging, false by default.
Additionally, per-request logging may be coming from the Rack logger itself, and the docs say (ctrl-f "logging") that you need to set :logging, nil (which for some reason is different than set :logging, false) to disable it entirely.
There was a problem hiding this comment.
I tested the things you suggested, but I can't get it to be quiet or disable logging with these options either 🤷♂️
To test the diagnose reports for all integrations, submit the reports from the integrations to a mock server for the test suite. This mock server will receive the report and parse the JSON. Then we can use that parsed report to test on it in the diagnose tests. ## Changes to the specs I've added separate specs for report structure and values. I didn't want to mix them in the output specs even though they test the same values, but I don't want to spec to fail if only the output or the report is wrong. I've added a new `expect_report_for` helper to test the report itself, but this is not done yet, as it doesn't make sure that the entire report section structure matches. The specs now submit the diagnose report by default, and we have separate tests for what happens when we don't. This makes it easier to test the report values along side the output specs. ## Changes to the integrations To have the integrations submit to this mock server, they need to be configured with a new server endpoint. I've done this with the `APPSIGNAL_DIAGNOSE_ENDPOINT` environment variable. The integrations will need to be updated to listen to this environment variable and use its value as an endpoint instead of the default `appsignal.com/diag`. ## Output to STDOUT The diagnose specs now log Sinatra requests (from the mock server) to STDOUT. I've already set `logging` to `false`, but it still logs some information. Not sure how to disable that.
No integrations include an app section with anything in it. Only the Node.js integration sends an empty object for this section, so I'm removing it.
The diagnose CLIs in the Ruby and Elixir integrations wrap the diagnose report in a "diagnose" key. This is because of how the Rails endpoint was originally written. Update the diagnose tests, which were originally written against the Node.js integration, to fetch the report from the nested diagnose key. I've updated the Node.js integration to match.
tombruijn
force-pushed
the
transmitted-report
branch
from
9246eaa to
7c1bc97
Compare
Part of https://github.com/appsignal/integration-guide/issues/59
Rename expect_section helper
Rename the expect_section to expect_output_for. This name is more clear,
it asserts the output of the runner, for a specific section.
Submit diagnose reports to spec server
To test the diagnose reports for all integrations, submit the reports
from the integrations to a mock server for the test suite.
This mock server will receive the report and parse the JSON. Then we can
use that parsed report to test on it in the diagnose tests.
Changes to the specs
I've added separate specs for report structure and values. I didn't want
to mix them in the output specs even though they test the same values,
but I don't want to spec to fail if only the output or the report is
wrong.
I've added a new
expect_report_forhelper to test the report itself,but this is not done yet, as it doesn't make sure that the entire report
section structure matches.
The specs now submit the diagnose report by default, and we have
separate tests for what happens when we don't. This makes it easier to
test the report values along side the output specs.
Changes to the integrations
To have the integrations submit to this mock server, they need to be
configured with a new server endpoint. I've done this with the
APPSIGNAL_DIAGNOSE_ENDPOINTenvironment variable. The integrationswill need to be updated to listen to this environment variable and use
its value as an endpoint instead of the default
appsignal.com/diag.Output to STDOUT
The diagnose specs now log Sinatra requests (from the mock server) to
STDOUT. I've already set
loggingtofalse, but it still logs someinformation. Not sure how to disable that.
Remove app section from diagnose report
No integrations include an app section with anything in it. Only the
Node.js integration sends an empty object for this section, so I'm
removing it.
Fetch the report from nested diagnose key
The diagnose CLIs in the Ruby and Elixir integrations wrap the diagnose
report in a "diagnose" key. This is because of how the Rails endpoint
was originally written.
Update the diagnose tests, which were originally written against the
Node.js integration, to fetch the report from the nested diagnose key.
I've updated the Node.js integration to match.