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: avoid instrumentation of project if none is loaded, fixes #5065 #5066

Merged
merged 1 commit into from
Jul 4, 2023

Conversation

gilbertsoft
Copy link
Member

@gilbertsoft gilbertsoft commented Jul 3, 2023

The Issue

How This PR Solves The Issue

Avoids instrumentation of the project if none is available.

Manual Testing Instructions

Run ddev --version in a folder without DDEV config.

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@gilbertsoft gilbertsoft requested a review from a team as a code owner July 3, 2023 18:00
@github-actions github-actions bot added the bugfix label Jul 3, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Shouldn't we also fix the place where it uses the invalid data? pkg/ddevapp/ddevapp.go:371

omitted := app.OmitContainersGlobal

Seems like that also should have been initialized? Or is it the app itself that doesn't exist here?

@gilbertsoft
Copy link
Member Author

Shouldn't we also fix the place where it uses the invalid data? pkg/ddevapp/ddevapp.go:371

omitted := app.OmitContainersGlobal

Seems like that also should have been initialized? Or is it the app itself that doesn't exist here?

Yes, app does not exist, that's the problem here.

@stasadev
Copy link
Member

stasadev commented Jul 3, 2023

$ ddev version
 ITEM             VALUE                                         
 DDEV version     v1.22.0-alpha3-30-gb57c5900                   
 architecture     amd64                                         
 db               ddev/ddev-dbserver-mariadb-10.4:v1.22.0-beta1 
 ddev-ssh-agent   ddev/ddev-ssh-agent:v1.22.0-beta1             
 docker           24.0.2                                        
 docker-compose   v2.18.1                                       
 docker-platform  dev                                           
 mutagen          0.17.1                                        
 os               linux                                         
 router           traefik:v2.10                                 
 web              ddev/ddev-webserver:v1.22.0-beta1             
 
Instrumentation is opted in, but AmplitudeAPIKey is not available. This usually means you have a locally-built ddev binary or one from a PR build. It's not an error. Please report it if you're using an official release build.

Looks like it is impossible to test this build from PR.

I tried my local build instead. I placed some dummy values ​​in pkg/versionconstants/versionconstants.go and configured the Makefile.

I use OpenSnitch to inspect my internet traffic and I see api2.amplitude.com in my connections, and DDEV works if I disable the internet.

No more errors 👍

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks for getting to the bottom of this!

@rfay rfay merged commit 8e78edd into ddev:master Jul 4, 2023
23 checks passed
@rfay rfay deleted the fix/amplitude-error branch July 4, 2023 23:01
@rfay rfay mentioned this pull request Jul 4, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants