Skip to content

Don't validate the JSON connection string if APPLICATIONINSIGHT__CONNECTION_STRING#3759

Closed
jeanbisutti wants to merge 1 commit into
mainfrom
jean/json-file-connection-string-validation
Closed

Don't validate the JSON connection string if APPLICATIONINSIGHT__CONNECTION_STRING#3759
jeanbisutti wants to merge 1 commit into
mainfrom
jean/json-file-connection-string-validation

Conversation

@jeanbisutti

@jeanbisutti jeanbisutti commented Jun 25, 2024

Copy link
Copy Markdown
Contributor

Because the connection string from the APPLICATIONINSIGHT__CONNECTION_STRING environment variable takes precedence over the connection string coming from the JSON file.

Fix #3707

&& !replacedConnectionString.startsWith("InstrumentationKey=")
&& config.connectionString.equals(replacedConnectionString)) {
&& config.connectionString.equals(replacedConnectionString)
&& System.getenv(APPLICATIONINSIGHTS_CONNECTION_STRING_ENV) == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe it's better to check env var before loading from the file?

It's not needed to load from the file when env var is present.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To do this with a code that is easy to read, https://github.com/microsoft/ApplicationInsights-Java/blob/5481277381728dcbc985117da619e61225be2237/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/configuration/ConfigurationBuilder.java#L757C9-L757C39 should be called before loading from a file. It seems that it would require to change several lines of code and check carefully that it does not break anything. In this PR, I'd rather make the minimal change and be confident to not break something. An unecessary read from a file may not be a user issue.

@heyams

heyams commented Jul 23, 2024

Copy link
Copy Markdown
Contributor

this is replaced by #3794, thus closing.

@heyams heyams closed this Jul 23, 2024
@trask trask deleted the jean/json-file-connection-string-validation branch July 23, 2025 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

APPLICATIONINSIGHTS_CONNECTION_STRING does not work in 3.5.x, works in 3.4.x

3 participants