elastic / hey-apm

Basic load generation for apm-server built on hey
Apache License 2.0
16 stars 16 forks source link

STACK version implementation does not work anymore with go modules #153

Closed v1v closed 4 years ago

v1v commented 4 years ago

The current daily hey-apm run does require to fetch the version for the stack from the previous implementation with the vendor:

For the time being, I'll use the hardcoded value 8.0.0-SNAPSHOT

simitt commented 4 years ago

We might be able to use the version from the healthcheck endpoint of the APM Server.

axw commented 4 years ago

We could alternatively use https://github.com/elastic/apm-server/blob/04d3af8222d8f6363ebfe6c4ef1ab5d70b5024b9/testing/environments/snapshot.yml#L6

However, it's not clear to me what the correct behaviour of the STACK_VERSION parameter is:

parameters {
  ...
  string(name: 'STACK_VERSION', defaultValue: '', description: 'Stack version Git branch/tag to use. Default behavior uses the apm-server@master version.')
}

I don't see how using a branch or tag for STACK_VERSION would work. The value is used in docker-compose.yml (via the environment variable mentioned below), which isn't going to accept either of those is it?

There's an environment variable with the same name, which is initialised to the parameter value. However, there's also this script:

if (params.STACK_VERSION.trim()) {
    env.STACK_VERSION = getVersion() + '-SNAPSHOT'
}

That seems backwards? Shouldn't it be checking that the result of trim is falseish, and then using getVersion for the default value?

So... perhaps we should just set the default parameter value to 8.0.0-SNAPSHOT, and then remove the script.

v1v commented 4 years ago

That seems backwards? Shouldn't it be checking that the result of trim is falseish, and then using getVersion for the default value?

Indeed! I guess there is a bug in the implementation

So I actually raised https://github.com/elastic/hey-apm/pull/154 to support some other parameters, and I took the chance to tidy up the getVersion to just use the hardcoded value, so far it's pointing out to 8.0.0-SNAPSHOT.

if the version is somewhere elser rather than in a running apm-server then it will be much simpler , otherwise, I'd say using a hardcoded version could be the way to go from simplicity.

axw commented 4 years ago

Looks good - this can be closed now right?

graphaelli commented 4 years ago

closed by #154