Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Commented Out Code for Azure Native Monitoring #464

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add Commented Out Code for Azure Native Monitoring #464

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
686a984
Add Commented Out Code for Azure Monitor Enablement
mattmccleary Jul 19, 2024
955c311
Minor Formatting
mattmccleary Jul 19, 2024
4c11773
Formatting
mattmccleary Jul 19, 2024
feb068b
Removing Secret from AppSettings.json
mattmccleary Jul 20, 2024
84614cf
Add conditional statements to each service
mattmccleary Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/eShop.AppHost/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,14 @@

var launchProfileName = ShouldUseHttpForEndpoints() ? "http" : "https";

// To configure Azure Monitor Application Insights, add the following:
// var insights = builder.AddConnectionString("myInsightsResource", "APPLICATIONINSIGHTS_CONNECTION_STRING");
Copy link
Member

Choose a reason for hiding this comment

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

This can be replaced with a conditional such as

var appInsights = (builder.Configuration.GetConnectionString("myAppInsightsResource") is not null)? 
    builder.AddConnectionString(
        "myAppInsightsResource",
        "APPLICATIONINSIGHTS_CONNECTION_STRING")
    : null;

// You also need to add .WithReference(insights) to each service that should be monitored.

// Services
var identityApi = builder.AddProject<Projects.Identity_API>("identity-api", launchProfileName)
.WithExternalHttpEndpoints()
// .WithReference(insights) // Sample reference to Azure Monitor Application Insights. Copy/paste for each service.
Copy link
Member

Choose a reason for hiding this comment

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

I was going to add an overload to have a conditional reference, but it already seems to be there, so the following could be added to each app and not need to be commented out. It should silently ignore it if the resource is null.

    .WithReference(appInsights,null,true);

@mitchdenny - It turns out this doesn't work as the resource parameter isn't nullable. Is there a cleaner workaround?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah the .WithReference doesn't like that appInsights could be null.
image

Copy link
Member

@mitchdenny mitchdenny Jul 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why not just use a conditional? I'm not sure we want to invent a new API for this?

Copy link
Member

Choose a reason for hiding this comment

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

@mitchdenny conditionals don't fit nicely into the chained builder pattern, unless there is something I don't know?

.WithReference(identityDb);

var identityEndpoint = identityApi.GetEndpoint(launchProfileName);
Expand Down
3 changes: 2 additions & 1 deletion src/eShop.AppHost/appsettings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
}
},
"ConnectionStrings": {
//"OpenAi": "Endpoint=xxxx;Key=xxxx"
// "OpenAi": "Endpoint=xxxx;Key=xxxx"
// "myInsightsResource": "REPLACE_WITH_YOUR_INSTRUMENTATION_KEY" // Copy/paste from your Application Insights Resource in Azure Portal
mattmccleary marked this conversation as resolved.
Show resolved Hide resolved
}
}
7 changes: 7 additions & 0 deletions src/eShop.ServiceDefaults/Extensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ private static IHostApplicationBuilder AddOpenTelemetryExporters(this IHostAppli
builder.Services.ConfigureOpenTelemetryTracerProvider(tracing => tracing.AddOtlpExporter());
}

// Uncomment to send telemetry Azure Monitor Application Insights
// if (!string.IsNullOrEmpty(builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"]))
// {
// builder.Services.AddOpenTelemetry().UseAzureMonitor();
// }
// Under eShop.ServiceDefaults, add the Nuget Package "Azure.Monitor.OpenTelemetry.AspNetCore" to the project

return builder;
}

Expand Down
Loading