Skip to content

Upgrade AzureMonitor to Microsoft.ApplicationInsights 3.x - #566

Open
okalangkenneth wants to merge 1 commit into
ActiveLogin:mainfrom
okalangkenneth:feature/applicationinsights-v3-556
Open

okalangkenneth wants to merge 1 commit into
ActiveLogin:mainfrom
okalangkenneth:feature/applicationinsights-v3-556

Conversation

@okalangkenneth

Copy link
Copy Markdown

Fixes #556

Application Insights 3.x removed the metrics parameter from TrackEvent and TrackException, so BankIdApplicationInsightsEventListener threw MissingMethodException when an app used 3.x. This upgrades the package to 3.x as agreed in the issue.

Changes

AzureMonitor package

  • Microsoft.ApplicationInsights 2.23.0 → 3.1.2
  • Listener uses TrackEvent(name, properties) and TrackException(exception, properties)
  • AL_User_AgeHint is sent as a string property (invariant culture) in customDimensions, still gated by LogUserPersonalIdentityNumberHints
  • Connection string overload keeps new TelemetryConfiguration() (not the shared CreateDefault()), and the configuration and TelemetryClient are created once via Lazy<TelemetryClient> since the listener is transient

Docs

  • BREAKINGCHANGES.md: new 13.0.0 section (heading marked TBD, please adjust) covering the 3.x requirement, the connection string requirement and the age hint move, with before/after KQL
  • monitor.md: "Average age" query reads customDimensions
  • bankid.md: InstrumentationKey wording changed to connection string

Samples

  • Microsoft.ApplicationInsights.AspNetCore 2.23.0 → 3.1.2, and Microsoft.Extensions.Logging.ApplicationInsights removed from the ServerSample (no 3.x release; Microsoft's migration guide says to remove it)
  • 3.x throws at startup without a connection string, so the samples now only register Application Insights and the event listener when one is configured. appsettings use ConnectionString instead of InstrumentationKey, with fallback to APPLICATIONINSIGHTS_CONNECTION_STRING
  • ServerSample _LayoutMaster.cshtml only renders the JavaScript snippet when it's registered
  • AzureProvisioningSample ARM template sets APPLICATIONINSIGHTS_CONNECTION_STRING instead of APPINSIGHTS_INSTRUMENTATIONKEY

Testing

  • Solution builds with 0 warnings, all 319 tests pass
  • Standalone.MvcSample starts and serves pages with no connection string configured
  • Ran Standalone.MvcSample against a test Application Insights resource with the simulated BankID environment, once with the default overload (TelemetryClient from DI) and once with the connection string overload. Both exported all events, and CollectCompleted / AspNetAuthenticateSuccess carried AL_User_AgeHint = 27 in customDimensions for test PIN 990807-2391:
ActiveLogin_BankId

This branch has not been deployed

No deployments
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.

Upgrade ActiveLogin.Authentication.BankId.AzureMonitor to use Microsoft.ApplicationInsights v3

1 participant