Add streams support in Generic Journey #279
Conversation
| .disableFollowRedirect | ||
| // Mimic real Chrome browser | ||
| .maxConnectionsPerHostLikeChrome |
There was a problem hiding this comment.
Gatling should allow up to 6 connections per host name by default, but I thought it is better to use Chrome value for visibility. But it basically changes nothing https://github.com/gatling/gatling/blob/045603bc5c9d4466cb97d1e54902451f9b18dadd/gatling-http/src/main/scala/io/gatling/http/protocol/HttpProtocolBuilder.scala#L108
There was a problem hiding this comment.
That's a good idea but I wonder whether we should use it as it's a deprecated property?
There was a problem hiding this comment.
Funny I missed that comment. I will just remove it, let's use the default value
| val pauseDuration = | ||
| Math.max(0L, request.timestamp.getTime - priorTimestamp.getTime) | ||
| Math.max(0L, stream.startTime.getTime - priorDate.getTime) | ||
| if (pauseDuration > 0L) { | ||
| steps = steps.pause(pauseDuration.toString, TimeUnit.MILLISECONDS) | ||
| } |
There was a problem hiding this comment.
I hope that using previous stream end time (point in time when the last request was finished) and the next stream start time (point in time when the first request was started) difference is valid for pause between requests?
There was a problem hiding this comment.
Seems reasonable to me.
| @@ -59,26 +59,27 @@ object ApiCall { | |||
| if (request.headers.contains("Kbn-Version")) | |||
| defaultHeaders + ("Kbn-Version" -> config.version) | |||
| else defaultHeaders | |||
| val path = request.path.replaceAll(".+?(?=\\/bundles)", ""); | |||
There was a problem hiding this comment.
Since from build to build full path for bundles will have different prefix (sequence of digits), I think it is better to cut digits part and make it consistent in report. thoughts?
> GET /bundles/plugin/kibanaOverview/kibana/kibanaOverv... (OK=17 KO=0 )
> GET /bundles/plugin/newsfeed/kibana/newsfeed.plugin.j... (OK=17 KO=0 )
> GET /bundles/plugin/ux/8.0.0/ux.plugin.js (OK=17 KO=0 )
Alternatively, the full one is /9007199254740991/bundles/plugin/security/8.0.0/security.plugin.js
There was a problem hiding this comment.
Your suggestion makes sense to me as it keeps paths consistent over time. We could probably pick a different name for path to convey that this is only used for reporting and not the actual full request path used on the wire.
| .disableFollowRedirect | ||
| // Mimic real Chrome browser | ||
| .maxConnectionsPerHostLikeChrome |
There was a problem hiding this comment.
That's a good idea but I wonder whether we should use it as it's a deprecated property?
| config: KibanaConfiguration | ||
| ): ChainBuilder = { | ||
| val defaultHeaders = request.headers.-( | ||
| // Workaround for https://github.com/gatling/gatling/issues/3783 | ||
| val httpParentRequest = httpRequest(requests(0).http, config) |
There was a problem hiding this comment.
Instead of using array indices we could also use .head and .tail which would improve readability IMHO.
| @@ -59,26 +59,27 @@ object ApiCall { | |||
| if (request.headers.contains("Kbn-Version")) | |||
| defaultHeaders + ("Kbn-Version" -> config.version) | |||
| else defaultHeaders | |||
| val path = request.path.replaceAll(".+?(?=\\/bundles)", ""); | |||
There was a problem hiding this comment.
Your suggestion makes sense to me as it keeps paths consistent over time. We could probably pick a different name for path to convey that this is only used for reporting and not the actual full request path used on the wire.
| val pauseDuration = | ||
| Math.max(0L, request.timestamp.getTime - priorTimestamp.getTime) | ||
| Math.max(0L, stream.startTime.getTime - priorDate.getTime) | ||
| if (pauseDuration > 0L) { | ||
| steps = steps.pause(pauseDuration.toString, TimeUnit.MILLISECONDS) | ||
| } |
There was a problem hiding this comment.
Seems reasonable to me.
| val defaultHeaders = request.headers.-( | ||
| // Workaround for https://github.com/gatling/gatling/issues/3783 | ||
| val httpParentRequest = httpRequest(requests.head.http, config) | ||
| val children = requests.drop(1); |
There was a problem hiding this comment.
Nit: You can use .tail here.
|
I'm merging this PR and will address #281 in a separate effort and it requires some investigation and extra testing. We still can run without static bundle requests and do not face Java heap space |
Summary
Updating GenericJourney in order to support new JSON structure and be able to run requests in parallel.
This PR comes along with elastic/kibana/pull/138263 and can be tested as soon as this one is merged.
Checklist
Delete any items that are not applicable to this PR.