Skip to content

Logging configuration for Application Insights#1203

Merged
josielsouzanordcloud merged 1 commit intomainfrom
DTOSS-12264-app-insights-review
Mar 23, 2026
Merged

Logging configuration for Application Insights#1203
josielsouzanordcloud merged 1 commit intomainfrom
DTOSS-12264-app-insights-review

Conversation

@swebberuk
Copy link
Copy Markdown
Contributor

@swebberuk swebberuk commented Mar 19, 2026

Description

Changes for logging to Application Insights, now I have access to Azure to check. This builds on top of what was already merged in #1153


To test I added these log lines to the show clinic page:

    logger.critical("Logging at CRITICAL")
    logger.error("Logging at ERROR")
    logger.warning("Logging at WARNING")
    logger.info("Logging at INFO")
    logger.debug("Logging at DEBUG")

Then in Application Insights I could see them. Except the DEBUG message, as log level is INFO:

image

When I forced an error (by trying to check information of an appointment that had not been started) I could see the exception details in Application Insights. The Custom Properties included user and correlation_id:

image

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12264

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 19, 2026

The review app at this URL has been deleted:
https://pr-1203.manage-breast-screening.non-live.screening.nhs.uk

@swebberuk swebberuk force-pushed the DTOSS-12264-app-insights-review branch from 2985fc3 to 98658ac Compare March 19, 2026 16:38
@sonarqubecloud
Copy link
Copy Markdown

# APPLICATIONINSIGHTS_CONNECTION_STRING environment variable.
configure_azure_monitor(
# Set the namespace for the logger in which you would like to collect telemetry for if you are collecting logging telemetry. This is imperative so you do not collect logging telemetry from the SDK itself.
logger_name=self.logger_name,
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.

No longer specify logger_name in call to configure_azure_monitor. logger_name was a blocker for full log visibility

The logger_name parameter tells the SDK to attach its App Insights log handler only to the "insights-logger" logger. Every other logger is unaffected.

"APPLICATIONINSIGHTS_LOGGER_NAME", "insights-logger"
)
os.environ.setdefault("OTEL_SERVICE_NAME", self.logger_name)
os.environ.setdefault("OTEL_SERVICE_NAME", "manage-breast-screening")
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.

Used more descriptive name, rather than insights-logger.

@@ -305,29 +305,12 @@ def list_env(key):
"loggers": {
Copy link
Copy Markdown
Contributor Author

@swebberuk swebberuk Mar 19, 2026

Choose a reason for hiding this comment

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

No longer specify "handlers": ["console"] and "propagate": False - instead let them propagate up and be handled by the root logger, which is configured to use "handlers": ["console"].

"handlers": ["console"],
"level": "INFO",
"propagate": False,
"filters": ["suppress_duplicate_exceptions"],
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.

Keeping "filters": ["suppress_duplicate_exceptions"] to avoid duplicate logging of 500 errors that will of been logged separately by ExceptionLoggingMiddleware

@swebberuk swebberuk changed the title [wip] Experiment with application insights Logging configuration for Application Insights Mar 19, 2026
@swebberuk swebberuk marked this pull request as ready for review March 19, 2026 16:51
@swebberuk swebberuk requested a review from gpeng March 19, 2026 16:52
Copy link
Copy Markdown
Contributor

@gpeng gpeng left a comment

Choose a reason for hiding this comment

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

Looks good. Not part of this PR but I wonder if we'd be better logging the user ID rather than firstname_lastname.

@josielsouzanordcloud josielsouzanordcloud merged commit 89af261 into main Mar 23, 2026
13 checks passed
@josielsouzanordcloud josielsouzanordcloud deleted the DTOSS-12264-app-insights-review branch March 23, 2026 09:49
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.

3 participants