Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(#237): Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters #238

Closed
wants to merge 1 commit into from

Conversation

andreev-io
Copy link

Description

Replace gauges used for the forward module with counters.

Related Issue

#237

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

@andreev-io andreev-io requested a review from a team as a code owner April 8, 2024 12:12
@andreev-io andreev-io changed the title Fix #237: Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters [DRAFT] Fix #237: Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters Apr 8, 2024
@andreev-io andreev-io changed the title [DRAFT] Fix #237: Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters Fix #237: Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters Apr 8, 2024
@andreev-io andreev-io marked this pull request as draft April 8, 2024 12:57
@andreev-io
Copy link
Author

Hey folks! What's the right way to build this locally? I'm on a Mac but can spin up a VM as needed.

@rbtr
Copy link
Collaborator

rbtr commented Apr 8, 2024

Hey folks! What's the right way to build this locally? I'm on a Mac but can spin up a VM as needed.

Do you want to build images? You can use the Makefile, eg make retina-image

@andreev-io
Copy link
Author

@microsoft-github-policy-service agree company="ClickHouse"

@andreev-io
Copy link
Author

@rbtr Likely an issue on my end, but perhaps you've seen these before?

Error from building retina-image:

➜  retina-fork git:(ilya/l4-protocol) ✗ make retina-image
echo "Building for linux/amd64"
Building for linux/amd64
for target in init agent; do \
                echo "Building for $target"; \
                if [ "$target" = "init" ]; then \
                        image_name=andreev-io/retina/retina-init; \
                else \
                        image_name=andreev-io/retina/retina-agent; \
                fi; \
                /Library/Developer/CommandLineTools/usr/bin/make container-docker \
                                PLATFORM=linux/amd64 \
                                DOCKERFILE=controller/Dockerfile.controller \
                                REGISTRY=ghcr.io \
                                IMAGE=$image_name \
                                VERSION=3edcd38 \
                                TAG=3edcd38-linux-amd64 \
                                APP_INSIGHTS_ID="" \
                                CONTEXT_DIR=/Users/ilyaandreev/Documents/GitHub/retina-fork \
                                TARGET=$target; \
        done
Building for init
if docker buildx inspect retina > /dev/null 2>&1; then \
                echo "Buildx instance retina already exists."; \
        else \
                echo "Creating buildx instance retina..."; \
                docker buildx create --name retina --use --platform $(echo "linux/amd64 linux/arm64 windows/amd64" | tr ' ' ','); \
                docker buildx use retina; \
                echo "Buildx instance retina created."; \
        fi;
Buildx instance retina already exists.
os=$(echo linux/amd64 | cut -d'/' -f1); \
        arch=$(echo linux/amd64 | cut -d'/' -f2); \
        image_name=$(basename andreev-io/retina/retina-init); \
        image_metadata_filename="image-metadata-$image_name-3edcd38-linux-amd64.json"; \
        touch $image_metadata_filename; \
        echo "Building $image_name for $os/$arch "; \
        docker buildx build \
                 \
                --platform linux/amd64 \
                --metadata-file=$image_metadata_filename \
                -f controller/Dockerfile.controller \
                --build-arg VERSION=3edcd38  \
                --build-arg GOOS=$os \
                --build-arg GOARCH=$arch \
                --build-arg APP_INSIGHTS_ID= \
                --target=init \
                -t ghcr.io/andreev-io/retina/retina-init:3edcd38-linux-amd64 \
                /Users/ilyaandreev/Documents/GitHub/retina-fork
Building retina-init for linux/amd64 
[+] Building 7.3s (1/1) FINISHED                                                                                                                                                             docker-container:retina
 => ERROR [internal] booting buildkit                                                                                                                                                                           7.3s
 => => pulling image moby/buildkit:buildx-stable-1                                                                                                                                                              7.3s
 => => creating container buildx_buildkit_retina0                                                                                                                                                               0.0s
------
 > [internal] booting buildkit:
------
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: Error response from daemon: No such image: moby/buildkit:buildx-stable-1
make[1]: *** [container-docker] Error 1
Building for agent
if docker buildx inspect retina > /dev/null 2>&1; then \
                echo "Buildx instance retina already exists."; \
        else \
                echo "Creating buildx instance retina..."; \
                docker buildx create --name retina --use --platform $(echo "linux/amd64 linux/arm64 windows/amd64" | tr ' ' ','); \
                docker buildx use retina; \
                echo "Buildx instance retina created."; \
        fi;
Buildx instance retina already exists.
os=$(echo linux/amd64 | cut -d'/' -f1); \
        arch=$(echo linux/amd64 | cut -d'/' -f2); \
        image_name=$(basename andreev-io/retina/retina-agent); \
        image_metadata_filename="image-metadata-$image_name-3edcd38-linux-amd64.json"; \
        touch $image_metadata_filename; \
        echo "Building $image_name for $os/$arch "; \
        docker buildx build \
                 \
                --platform linux/amd64 \
                --metadata-file=$image_metadata_filename \
                -f controller/Dockerfile.controller \
                --build-arg VERSION=3edcd38  \
                --build-arg GOOS=$os \
                --build-arg GOARCH=$arch \
                --build-arg APP_INSIGHTS_ID= \
                --target=agent \
                -t ghcr.io/andreev-io/retina/retina-agent:3edcd38-linux-amd64 \
                /Users/ilyaandreev/Documents/GitHub/retina-fork
Building retina-agent for linux/amd64 
[+] Building 6.9s (1/1) FINISHED                                                                                                                                                             docker-container:retina
 => ERROR [internal] booting buildkit                                                                                                                                                                           6.9s
 => => pulling image moby/buildkit:buildx-stable-1                                                                                                                                                              6.9s
 => => creating container buildx_buildkit_retina0                                                                                                                                                               0.0s
------
 > [internal] booting buildkit:
------
WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load
ERROR: Error response from daemon: No such image: moby/buildkit:buildx-stable-1
make[1]: *** [container-docker] Error 1
make: *** [retina-image] Error 2

Error from building retina-binary:
Do I need a go.work file for this?

➜  retina-fork git:(ilya/l4-protocol) ✗ make retina-binary
export CGO_ENABLED=0 && \
        go generate ./... && \
        go build -v -o /Users/ilyaandreev/Documents/GitHub/retina-fork/output/linux_amd64/retina/retina -gcflags="-dwarflocationlists=true" -ldflags "-X main.version=3edcd38 -X main.applicationInsightsID=""" /Users/ilyaandreev/Documents/GitHub/retina-fork/controller/main.go
package command-line-arguments
        imports github.com/microsoft/retina/pkg/managers/controllermanager
        imports github.com/microsoft/retina/pkg/managers/pluginmanager
        imports github.com/microsoft/retina/pkg/plugin/registry: build constraints exclude all Go files in /Users/ilyaandreev/Documents/GitHub/retina-fork/pkg/plugin/registry
make: *** [retina-binary] Error 1

@rbtr rbtr added lang/go The Go Programming Language type/fix Fixes something area/metrics priority/1 P1 labels Apr 8, 2024
@rbtr
Copy link
Collaborator

rbtr commented Apr 8, 2024

imports github.com/microsoft/retina/pkg/plugin/registry: build constraints exclude all Go files in /Users/ilyaandreev/Documents/GitHub/retina-fork/pkg/plugin/registry

This problem with compiling the binary is likely due to your host platform not being supported by this package (only Linux and Windows build constraints are supported):
image

Does anything change if you set env vars for GOOS=linux, GOARCH=amd64? I don't have access to a Mac to test but @vakalapa is going to be checking another issue and might be able to test this also.

No such image: moby/buildkit:buildx-stable-1

This one I am more confused about, as that is a Moby infra image that should definitely exist...not sure how to fix this 🤔

WARNING: No output specified with docker-container driver. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load

Is resolved by adding a BULIDX_ACTION such as make retina-image BUILDX_ACTION=--load

Copy link
Contributor

@anubhabMajumdar anubhabMajumdar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the review!

Copy link

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions bot added the meta/waiting-for-author Blocked and waiting on the author label May 23, 2024
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this May 30, 2024
@nddq nddq reopened this May 30, 2024
@nddq nddq changed the title Fix #237: Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters fix(#237): Use Prometheus type Counter instead of Gauge for forward module's packet and byte counters May 30, 2024
@github-actions github-actions bot removed the meta/waiting-for-author Blocked and waiting on the author label May 31, 2024
Copy link

This PR will be closed in 7 days due to inactivity.

@github-actions github-actions bot added the meta/waiting-for-author Blocked and waiting on the author label Jun 30, 2024
Copy link

github-actions bot commented Jul 7, 2024

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics lang/go The Go Programming Language meta/waiting-for-author Blocked and waiting on the author priority/1 P1 type/fix Fixes something
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants