Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 8, 2025

This PR addresses two key issues in the telemetry lesson that were making it difficult for workshop participants to follow along:

1. Replaced API Code Placeholder

The README.md contained a placeholder comment // ... API call logic ... instead of the actual working code implementation. This was error-prone and required participants to guess or look elsewhere for the complete code.

Before:

try 
{
    // ... API call logic ...
    stopwatch.Stop();
    // ... rest of implementation
}

After:

try 
{
    // Create an exception every 5 calls to simulate an error for testing
    if (forecastCount % 5 == 0)
    {
        throw new Exception("Random exception thrown by NwsManager.GetForecastAsync");
    }

    var zoneIdSegment = HttpUtility.UrlEncode(zoneId);
    var forecasts = await httpClient.GetFromJsonAsync<ForecastResponse>($"zones/forecast/{zoneIdSegment}/forecast", options);
    stopwatch.Stop();
    
    // Record the request duration
    NwsManagerDiagnostics.forecastRequestDuration.Record(stopwatch.Elapsed.TotalSeconds);
    activity?.SetTag("request.success", true);

    var result = forecasts
        ?.Properties
        ?.Periods
        ?.Select(p => (Forecast)p)
        .ToArray() ?? [];

    logger.LogInformation(
        "📊 Retrieved forecast for zone {ZoneId} in {Duration:N0}ms with {PeriodCount} periods",
        zoneId,
        stopwatch.Elapsed.TotalMilliseconds,
        result.Length
    );

    return result;
}

2. Added Complete Cache Metrics Documentation

The lesson defined cache_hits_total and cache_misses_total metrics but didn't properly document or implement their usage. Added a comprehensive "Implementing Cache Metrics" section that:

  • Shows how to properly track both cache hits and misses using cache.TryGetValue() pattern
  • Explains the difference between cache hit and miss scenarios
  • Demonstrates activity tracing tags for distributed tracing
  • Provides structured logging examples for cache performance monitoring

3. Enhanced Lesson Code Implementation

Updated the GetZonesAsync method in the lesson code to properly implement cache hit tracking:

// Check if data exists in cache first
if (cache.TryGetValue("zones", out Zone[]? cachedZones))
{
    // Record cache hit
    NwsManagerDiagnostics.cacheHitCounter.Add(1);
    activity?.SetTag("cache.hit", true);
    
    logger.LogInformation("📈 Retrieved {ZoneCount} zones from cache (cache hit)", cachedZones?.Length ?? 0);
    return cachedZones;
}

These changes make the telemetry lesson more complete and easier to follow, providing participants with working code they can copy-paste without errors.

Fixes #185.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • aka.ms
    • Triggering command: curl -I -sSL --retry 5 --retry-delay 2 --connect-timeout 15 REDACTED (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] Improve Telemetry Lesson Improve Telemetry Lesson: Replace API placeholder and add cache metrics documentation Sep 8, 2025
@Copilot Copilot AI requested a review from jongalloway September 8, 2025 20:14
Copilot finished work on behalf of jongalloway September 8, 2025 20:14
@jongalloway jongalloway requested a review from Copilot September 9, 2025 06:40
Copy link
Contributor

@Copilot 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

This PR enhances the telemetry lesson by replacing incomplete placeholder code with fully functional implementations and adding comprehensive cache metrics documentation. The changes make the workshop more complete and easier for participants to follow.

Key changes:

  • Replaced API placeholder comment with complete working code implementation including error simulation
  • Added comprehensive cache metrics implementation section with proper hit/miss tracking
  • Enhanced the GetZonesAsync method with activity tracing and structured logging

Reviewed Changes

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

File Description
workshop/Lesson-06-Telemetry/code/Api/Data/NwsManager.cs Implements cache hit tracking with activity tracing and improved logging
workshop/Lesson-06-Telemetry/README.md Replaces API placeholder with complete implementation and adds cache metrics documentation

@csharpfritz csharpfritz marked this pull request as ready for review October 2, 2025 18:40
@csharpfritz csharpfritz merged commit 97b8b5f into main Oct 2, 2025
10 checks passed
@csharpfritz csharpfritz deleted the copilot/fix-185 branch October 2, 2025 18:43
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.

Improve Telemetry Lesson

3 participants