Skip to content

Connection String env var - base, web, nlog#3134

Merged
harsimar merged 2 commits intomainfrom
harskaur/connectionString
Mar 3, 2026
Merged

Connection String env var - base, web, nlog#3134
harsimar merged 2 commits intomainfrom
harskaur/connectionString

Conversation

@harsimar
Copy link
Copy Markdown
Member

@harsimar harsimar commented Mar 3, 2026

This PR adds code to fall back to the env var for connection string if it isn't set via code for the BASE, WEB, NLogTarget packages.

Tested by setting a system env var then running example apps in repo with code config for connection string removed.

Copilot AI review requested due to automatic review settings March 3, 2026 18:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds connection-string resolution via the standard APPLICATIONINSIGHTS_CONNECTION_STRING environment variable when a connection string isn’t provided programmatically, aligning BASE and NLog target behavior with common Azure Monitor configuration patterns.

Changes:

  • NLog target (ApplicationInsightsTarget) now falls back to APPLICATIONINSIGHTS_CONNECTION_STRING if the configured layout renders empty/whitespace.
  • BASE TelemetryConfiguration.Build() now reads APPLICATIONINSIGHTS_CONNECTION_STRING when ConnectionString was not set in code, and avoids setting exporter options when the value is empty.
  • BASE InitializeFeatureReporter() now short-circuits when connectionString is empty (but this introduces a correctness risk; see comments).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
LOGGING/src/NLogTarget/ApplicationInsightsTarget.cs Adds env-var fallback for the NLog target connection string during initialization.
BASE/src/Microsoft.ApplicationInsights/Extensibility/TelemetryConfiguration.cs Adds env-var fallback during OTel SDK build and adjusts exporter option assignment / feature reporter initialization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Member

@mattsains-msft mattsains-msft left a comment

Choose a reason for hiding this comment

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

Other than the first LLM-generated comment, I think this PR is good to go. Will approve after that's resolved

@harsimar harsimar merged commit 9ca8e2a into main Mar 3, 2026
22 checks passed
@harsimar harsimar deleted the harskaur/connectionString branch March 3, 2026 21:43
harsimar added a commit that referenced this pull request Apr 1, 2026
Add changelog entries for #3137, #3134, and #3154.
Update SemanticVersionDate to 2026-04-01.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
harsimar added a commit that referenced this pull request Apr 1, 2026
Set version to 3.1.0, update SemanticVersionDate to 2026-04-01.
Add changelog entries for #3137, #3134, and #3154.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
harsimar added a commit that referenced this pull request Apr 1, 2026
Set version to 3.1.0, update SemanticVersionDate to 2026-04-01.
Add changelog entries for #3137, #3134, and #3154.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Apr 2, 2026
This was referenced Apr 5, 2026
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.

5 participants