feat: create new telemetry handle that supports connection strings #3729
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new telemetry client that supports connection strings for initializing Application Insights telemetry, along with corresponding tests.
- Updated go.mod to replace the ApplicationInsights-Go dependency.
- Added a new function, NewAITelemetryWithConnectionString, in telemetrywrapper.go to enable connection string based initialization.
- Expanded tests in telemetrywrapper_test.go to cover the new connection string initialization.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go.mod | Updated ApplicationInsights-Go dependency replacement. |
| aitelemetry/telemetrywrapper_test.go | Added tests for connection string initialization and error checks. |
| aitelemetry/telemetrywrapper.go | Added NewAITelemetryWithConnectionString function implementation. |
Comments suppressed due to low confidence (1)
aitelemetry/telemetrywrapper_test.go:93
- There is a spelling mistake in the error message; 'intializing' should be changed to 'initializing'.
t.Errorf("Error intializing AI telemetry:%v", err)
BeegiiK
commented
Jun 11, 2025
go.mod
Outdated
| @@ -188,6 +188,7 @@ require ( | |||
| ) | |||
|
|
|||
| replace ( | |||
| github.com/microsoft/ApplicationInsights-Go => github.com/beegiik/ApplicationInsights-Go v1.67.0 | |||
There was a problem hiding this comment.
This will be removed - was used to test
BeegiiK
changed the title
BeegiiK
force-pushed
the
connection-string
branch
2 times, most recently
from
d0708af to
56dd2e4
Compare
BeegiiK
force-pushed
the
connection-string
branch
from
987145b to
fa88a0b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reason for Change:
Currently, the telemetry handle can only work with instrumentation keys. If we want to use application insights in sovereign clouds, this would simply fail as the logging is shipped to a default ingestion endpoint and the
isPublicEnvironmentcheck.This PR introduces a new telemetry handle method that takes in a connection string, parses it, updates the configuration appropriately and initializes the new handle
Tested manually by updating DNC to use the new telemetry handle and verified that the logs were shipped to Kusto, with the connection string of the test application insights resource:
Issue Fixed:
#3730
Requirements:
Notes: