Save a copy of `grpc_health_probe` binary to the repository

[mic-max]

Feature Request

Save a copy of grpc_health_probe binary to the repository. 10MB.

Is your feature request related to a problem?

Each of our services downloads this health probe (and will soon use with Dockerfile HEALTHCHECK)) using wget. Example: https://github.com/open-telemetry/opentelemetry-demo-webstore/blob/main/src/adservice/Dockerfile#L35

Describe the solution you'd like:

Instead of downloading each time which a version has to be specified in each Dockerfile or possibly one main GRPC_HEALTH_PROBE_VERSION value in the .env file that we'd have to add to each service's environment section in the compose.yml file, we simply copy the file from /bin/ to the docker image.

Pros:

• Don't need to install wget in each service's build
• No external dependencies (network)
• Don't have to download 10MB file each new build (around 10 times), some issues have been seen with this command
• Simplifies the task of using the same version of the health probe
• Less boilerplate code

Cons:

• Checking in a binary is usually considered a no-no
• Git repository size increases (from 20MB to 30MB)

Describe alternatives you've considered.

If the Docker compose command is able to download this binary and it be copied to each image during the build process could alleviate most of the network download required from 100MB total to 10MB total. However, each clean install will still need network access to get this file, so the offline-friendly approach of including the binary to the repository seems worthwhile.

[austinlparker]

sgtm

[reyang]

This decision - once made - is hard to revert. The binary file will always sit in the commit history and be part of every git clone.

Maybe consider using submodule as the 1st step - put the binary in another repo, include it as a submodule here, and in the document mention git clone/pull with submodules. And after let's say 6 months or a year if we confirmed it's a great idea, make it to the main repo.

[wph95]

I personally recommend only downloading the grpc_health_probe binary locally at build time, rather than saving it in the github repo. p.s. We are working on https://github.com/open-telemetry/opentelemetry-demo-webstore/issues/78 So future users are more likely to use the pre-built image directly, rather than building it themselves.

[julianocosta89]

Hello all,

Do we have a final decision here? I'm asking because we have the following PRs from @mic-max: #93, #105, #116, #117 and #118. All of those have the download of grpc_health_probe binary removed from the Dockerfile

[mic-max]

Those PRs weren't meant to try to push one direction with this issue I just removed the wget commands since the downloaded binary was not being used anywhere.

To @reyang 's point of the binary always being in the git history I think this file will only ever be updated a few times in the life of the project so I'd be fine with ~40MB being added to a git clone for the benefit of not downloading the binary multiple times and removing that network dependency to the docker compose build process. Using a submodule changes how to clone git clone --recurse-submodules and we'd have to setup a new repo which does add a little more complexity to the project which I think should be avoided for the demo.

So future users are more likely to use the pre-built image directly, rather than building it themselves. - @wph95 I don't understand what the downside you're suggesting is here, would you care to elaborate?

[julianocosta89]

If we remove the wget grpc_health_probe from the Dockerfiles now, if we decide to not have the binary in the repo, whenever we write down the instructions for k8s we would need to bring it back in all Dockerfiles.

I agree that we are not using the grpc_health_probe ATM, but this will be used in the k8s manifests files.

[austinlparker]

Personally I'm not a fan of submodules. I'm also not entirely sure about this claim:

So future users are more likely to use the pre-built image directly, rather than building it themselves.

I can see plenty of cases where someone is running a mix of local + prebuilt images, especially if they're using this repo to experiment with OpenTelemetry.

As a compromise solution, would it be possible to create a preinstall/prebuild script that downloads and caches grpc_health_probe locally in a known location or create an image layer that all of the dockerfiles can pull from during their builds so that we avoid the overhead of downloading it multiple times per build?

[mic-max]

I appreciate all these suggestions and alternatives and they do seem good and better IMO than the existing method. However, I keep coming back to simply saving the binary to the repository.

Some reasons:

• No extra steps for building locally or via CI pipelines
• No network dependency to build images after cloning the repository
• No additional tool required such as wget in the building of the images
• Simplifies the maintenance of using the same version of the binary everywhere
• Git repository size increase with initial and infrequent updates to the binary seem like a minor inconvenience

[julianocosta89]

Hello again,

Sorry to keep coming back to this, but I think we should decide it as the PRs #105, #116, #117 and #118 got merged. All of them removed the grpc_health_probe download from the Dockerfiles.

Will we add the binary to the repo, or should we re-add the wget to the Dockerfiles?

[mic-max]

k8s we would need to bring it back in all Dockerfiles

@julianocosta89 to your comment would the local binary not suffice for this k8s scenario? Also, it doesn't look like we have much discussion here so I'll bring it up next Monday's meeting since there wasn't one this week. I will have this PR out of draft stage by then as well

[julianocosta89]

Once the grpc_health_probe is in the container we can call it from k8s without any issue. The issue is that if we remove the grpc_health_probe download from the Dockerfile and we do not agree in adding the binary into the repo, then we will need to add the download back to the Dockerfile again. I'm just trying to avoid the re-work.

[austinlparker]

Doesn't seem like we still need this?