Embed target server type

[alandipert]

Corresponding shinycannon PR: rstudio/shinycannon#36

Motivation

shinyloadtest supports recording and playing back sessions against a variety of
target server types, including RStudio Connect, Shiny Server and Shiny Server
Pro, and the "default" server run out of an R console session with
shiny::runApp().

While the recording.log produced by shinyloadtest::record_session() with
these servers is very similar, it's not similar in all cases for us to say
that the recording.log file is server-type agnostic. We have documented this
limitation in the past but we do not explicitly disallow it in
software.

Currently, if a user attempts to play a recording back against a different
server, shinycannon fails with an obscure error that will not indicate the
disparity. For example, below is what currently happens when a recording made
against a local R server is played back against an app in Connect:

2019-06-12 11:43:30.260 ERROR [thread01] - Playback failed: Status 404 received, expected 200, URL: https://beta.rstudioconnect.com/content/4215/font-awesome-5.
3.1/css/all.min.css, Response body: <h1>Not Found</h1>
java.lang.IllegalStateException: Status 404 received, expected 200, URL: https://beta.rstudioconnect.com/content/4215/font-awesome-5.3.1/css/all.min.css, Respon
se body: <h1>Not Found</h1>
        at com.rstudio.shinycannon.Event$Http.get(Events.kt:148)2019-06-12 11:43:30.262 INFO [thread01] - Stopped
2019-06-12 11:43:30.263 INFO [thread00] - Complete. Failed: 1, Done: 0

We can imagine a user wasting a lot of time debugging the above problem before
realizing they simply need to make a new recording against the different target
server.

It would be good if shinycannon produced a warning or error message if it
detected the target server differs from the type used to create the recording.

Initial Implementation Idea

The following things are true of the software in its current state:

• shinycannon can't produce a message because the original target server type is
  not included in recording.log.
• shinyloadtest recognizes if a target is shinyapps.io based on hostname and
  errors/exits early if so
• beyond that, shinyloadtest only distinguishes between RSC and SSP, and only
  when the app requires authentication
• shinycannon also only distinguishes between RSC and SSP, and only when the app
  requires authentication

So, at least the following enhancements are necessary:

• "Detection" code in shinycannon/shinyloadtest should be consolidated and
  extended so that it can distinguish reliably between RSC, SSP/SSO, R/Shiny
  (IDE/R console), and shinyapps.io.
  • Are SSP/SSO similar enough to be considered compatible? PR in existing state
    assumes "yes".
• As part of starting a recording, shinyloadtest should detect the server type
  and store that type in the recording file.
• As part of starting a load test, shinycannon should:
  1. Parse the server type from the recording file
  2. Detect the server type using its own mechanism
  3. Compare the parsed type with the detected type and produce an appropriate
     message/warning/error

Actual Implementation

1. The detection code in shinyloadtest was augmented and extended.
   • The biggest change here was the addition of detection for the R/Shiny type
     of application. Applications are considered R/Shiny if they don't appear
     to be RSC/SSP/SSO/shinyapps.io but the initial HTML includes a script
     tag with a src that ends with shiny.min.js.
   • A new detect.R file encapsulates all detection logic
2. shinyloadtest was modified to include the detection result in the
   recording.log file
   • The recording format was revisited and a few other changes were made. See
     below for details.
3. New detection code from shinyloadtest was ported to shinycannon:
   Detect.kt

Recording Format Changes

Previously, the first few lines of a recording looked like this:

# version: 1.0.0
# target: http://localhost:3969/
...

Now, the first few lines look like this:

# version: 1
# target_url: http://localhost:3969/
# target_type: R/Shiny
...

• version is not a semantic version number that could be thought to
  correspond to a release of shinyloadtest. Instead, it is an independent
  version specific to the recording format. Its semantics are defined by its
  interpretation by shinycannon.
• target was renamed to target_url
• A new field, target_type is introduced.

With this PR, shinycannon errors and exits when the recording format is old.

Whether or not to support the old recording format is an open question.

New behavior

shinyloadtest

Previously, if shinyloadtest didn't detect RSC or SSO/SSP, it would assume
R/Shiny and behave like a blind proxy, generating a log of traffic for any
arbitrary HTTP/WS app.

Now, the following kind of thing happens:

> record_session("https://google.com/", output_file = "goog.log")

 Error in servedBy(private$targetURL) :
  Target URL https://google.com/ does not appear to be a Shiny application.

New unrelated behaviors

There were a few opportunities to make some low-risk improvements unrelated to this feature while I was working in shinyloadtest, so I took the liberty.

• If an app is authenticated and the user cancels either the username or password dialogs, an error like the following error appears in the console:

 Error in private$initializeSessionCookies() : 
  Login aborted (username not provided)

• If a user provides bad credentials to an authenticated app, the following error appears in the console:

Error in handlePost(handle = curl::new_handle(), loginUrl = loginUrl,  : 
  Authentication failed

shinycannon

After these changes, shinycannon behaves in the following new ways (assuming the
recording is "new format"):

• If the recording format predates that proposed by this PR:

The following error is produced and shinycannon exits 1:

2019-06-12 14:26:56.978 ERROR [thread00] - Recording is in an unsupported format. Please update shinyloadtest, make a new recording, and try again

• If version > 1:

The following error is produced and shinycannon exits 1:

2019-06-12 13:51:54.246 ERROR [thread00] - Recording version 2 is newer than this version of shinycannon supports; please upgrade shinycannon

• If the detected server doesn't match the recorded server:

2019-06-12 13:51:54.243 WARN [thread00] - Recording made with 'R/Shiny' but target looks like 'RStudio Server Connect'

Shalu and Alan discussed whether or not shinycannon should error/exit here
instead of warn, and decided warning is better because the detection heuristics
are not perfect.

• If target detection fails:

The following error is produced and shinycannon exits 1:

alan@ubuntu:~/g/r/shinycannon$ java -jar target/shinycannon-1.0.0-jar-with-dependencies.jar ../shinyloadtest/local.log https://google.com/
[Fatal Error] :1:3: The markup in the document preceding the root element must be well-formed.
2019-06-12 14:11:41.694 ERROR [thread00] - Target URL https://google.com/ does not appear to be a Shiny application.

Open Questions

1. Should shinyloadtest::record_session() attempt to continue if the type of
   the target server can't be detected?
   • Yes: The detection rules are heuristics, and it's conceivable that a user
     with a complex setup or operating under weird circumstances that defeat
     our detection would want these tools to work regardless.
   • No: Detection failing for some reason is a rarer circumstance than
     accidentally using a recording against the wrong type of server and being
     flummoxed by errors
   • Alternative: Instead of erroring, record_session() could produce a
     warning message and encode a server type of "Unknown"/SERVER_TYPE$UNK.
2. Should shinycannon still work with old recordings?

[jcheng5]

Great writeup, Alan.

Regarding open questions:

1. I definitely think we should continue with a warning if detection fails.

2. I'd really like to continue supporting old files. I know there probably aren't a ton of people out there with existing recordings, but, the changes to the recording format are so superficial that it just seems rude to break existing user data.

[jcheng5]

I believe it's generally not a good idea to have top-level package variables that are objects created by other packages. At shinyloadtest install time, this rlang::env object will be serialized, but then rlang could be upgraded at a later date and then you could have a mismatch.

Maybe something like this instead:

enum_counter <- local({
  n <- 0L
  function() {
    (n <<- n + 1L)
  }
})

[jcheng5]

We'll probably need to walk through this PR in person, it's a bit much for me to digest without guidance.

[alandipert]

Can be: df[df$name == "SSP-XSRF",]

[alandipert]

Should also look for "/shiny.js" because minification can be turned off with an option