Skip to content

Conversation

jonhadfield
Copy link
Contributor

This new module enables table output of logs as queried by a log analytics workspace Kusto query.
Documentation updates with screenshot will follow in the next few days.

@Copilot Copilot AI review requested due to automatic review settings July 13, 2025 14:41
Copy link

@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 introduces a new Azure Logs module to WTF that enables users to display Azure Log Analytics query results in a table format. The module provides integration with Azure Monitor APIs to query log workspaces using KQL (Kusto Query Language) and displays the results in a formatted table widget.

Key Changes:

  • Adds complete Azure Log Analytics integration with authentication and query execution
  • Implements table-based data display with adaptive column widths and row truncation
  • Provides comprehensive test coverage across all module components

Reviewed Changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
modules/azurelogs/widget.go Core widget implementation with table rendering and data fetching logic
modules/azurelogs/session.go Azure authentication and session management
modules/azurelogs/query.go Query execution with concurrent client management
modules/azurelogs/config.go YAML configuration file parsing for query definitions
modules/azurelogs/settings.go Widget settings structure and initialization
go.mod Azure SDK dependencies and Go version update
app/widget_maker.go Widget registration in the main application
*_test.go files Comprehensive test coverage for all components
Comments suppressed due to low confidence (1)

modules/azurelogs/widget.go:166

  • Potential index out of bounds error. The code checks colIdx < len(colWidths) on line 166 but then uses colWidths[colIdx] without verifying the condition is still true. This could cause a panic if colWidths has fewer elements than expected.
			if colIdx < len(colWidths) && len(cellText) > colWidths[colIdx] {

Comment on lines 5 to 8
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"strings"
"time"

Copy link
Preview

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

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

[nitpick] The import of "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" should be grouped with other third-party imports, not placed between standard library and local imports. Consider moving it after line 9 with other Azure SDK imports.

Suggested change
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
"strings"
"time"
"strings"
"time"
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"

Copilot uses AI. Check for mistakes.

case string:
r = append(r, v)
case float64:
r = append(r, fmt.Sprintf("%.0f", v))
Copy link
Preview

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Using %.0f format for float64 values will truncate decimal places without rounding. Consider using proper rounding with fmt.Sprintf("%.0f", math.Round(v)) or a more explicit conversion like strconv.FormatFloat(v, 'f', 0, 64) to make the intention clear.

Copilot uses AI. Check for mistakes.

}

filename := file.Name()
if len(filename) > 5 && filename[len(filename)-5:] == ".yaml" {
Copy link
Preview

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

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

The file extension check is fragile and doesn't handle .yml files. Consider using strings.HasSuffix(filename, ".yaml") || strings.HasSuffix(filename, ".yml") or filepath.Ext(filename) for more robust file extension checking.

Suggested change
if len(filename) > 5 && filename[len(filename)-5:] == ".yaml" {
if strings.HasSuffix(filename, ".yaml") || strings.HasSuffix(filename, ".yml") {

Copilot uses AI. Check for mistakes.

case string:
return "\"" + val + "\""
case int:
return string(rune(val + '0'))
Copy link
Preview

Copilot AI Jul 13, 2025

Choose a reason for hiding this comment

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

This conversion is incorrect for integers greater than 9. string(rune(val + '0')) only works for single digits (0-9). For larger integers, this will produce incorrect characters. Use strconv.Itoa(val) instead.

Copilot uses AI. Check for mistakes.

@FelicianoTech
Copy link
Collaborator

Hi,

Some thoughts here.

  1. Can you dive into a bit of detail on how many users would use this module? I'm not familiar with Azure logs. It seems like it parses the results of a file and displays it. Would that data somehow change over time?
  2. This PR needs to be rebased and brought down to a single commit.
  3. This PR has a conflict.

We should probably walk through the first point before you spend time on 2 and 3.

@jonhadfield
Copy link
Contributor Author

jonhadfield commented Aug 7, 2025

Can you dive into a bit of detail on how many users would use this module? I'm not familiar with Azure logs. It seems like it parses the results of a file and displays it. Would that data somehow change over time?

In Azure, every service will natively send logs to a service called Azure Log Analytics Workspace. It's the equivalent of an ELK (Elasticsearch, Logstash, Kibana) stack where you, respectively, store, ship, and view/search the logs. Any users of Azure would benefit from this module by being able to see up-to-date results from predefined queries (Kusto is the name of the query language used in Log Analytics Workspace). If you're familiar with AWS, this is the equivalent of AWS CloudWatch. The rate of change is down to the service logging, but it's expected to change every few seconds.

How useful this would be to your user base is entirely down to how many would be using Azure for their cloud computing. If that's small/unlikely, then no need to include this and I'll happily continue with it in my private fork.

Will rebase asap.

@FelicianoTech FelicianoTech added this to the v0.47.0 milestone Aug 11, 2025
@FelicianoTech
Copy link
Collaborator

I need to give this a proper review still but I'm aiming to include this in the next minor release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants