Skip to content

Conversation

@erlendoksvoll
Copy link
Member

…ension method and log action

Description

Documentation

  • Doc updated

@erlendoksvoll
Copy link
Member Author

#197

This comment was marked as outdated.

@erlendoksvoll erlendoksvoll marked this pull request as ready for review July 8, 2025 07:39
Copy link
Contributor

@SondreJDigdir SondreJDigdir left a comment

Choose a reason for hiding this comment

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

Only minor feedback on the naming of the new method and route, but looks okie

@erlendoksvoll erlendoksvoll requested a review from Copilot July 10, 2025 06:54
Copy link

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 adds Parquet-based usage statistics per service context, enriches accreditation logging, and tracks service ownership.

  • Introduces GetLast24HoursPerServiceContext returning a ParquetSource and wires it up in FuncUsageStatistics as a new DigdirStatistics endpoint.
  • Extends service contexts with an Owner property and hardcodes owner values in ServiceContextService.
  • Adds a new DanLog overload in ILoggerExtension and injects ILogger into function classes.

Reviewed Changes

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

Show a summary per file
File Description
Dan.Core/Services/UsageStatisticsService.cs New method to query last 24h stats and build a Parquet payload.
Dan.Core/Services/ServiceContextService.cs Populates Owner on each ServiceContext entry.
Dan.Core/Models/ParquetSource.cs Defines ParquetSource and record model.
Dan.Core/FuncUsageStatistics.cs Adds DigdirStatistics function, Parquet serialization, logging.
Dan.Core/FuncAccreditationList.cs Injects ILogger, logs accreditation retrieval action.
Dan.Core/Extensions/ILoggerExtension.cs Adds new DanLog overload.
Dan.Core/Dan.Core.csproj Includes Parquet.Net package and local settings in output.
Dan.Common/Models/ServiceContext.cs Adds Owner data member to ServiceContext.
Dan.Common/Enums/LogAction.cs Adds AccreditationsRetrieved log action enum.
Comments suppressed due to low confidence (4)

Dan.Core/FuncUsageStatistics.cs:25

  • The original endpoint was annotated with NoAuthentication, but this attribute was removed. Re-add , NoAuthentication if public access without auth is intended.
    [Function("MonthlyUsageStatistics")]

Dan.Core/FuncUsageStatistics.cs:64

  • New DigdirStatistics endpoint should have corresponding unit or integration tests to verify Parquet serialization and response behavior.
    [Function("DigdirStatistics"), NoAuthentication]

Dan.Core/Services/UsageStatisticsService.cs:53

  • Consider adding XML documentation for this new public method to explain its purpose, query time range, and returned data structure.
    public async Task<ParquetSource> GetLast24HoursPerServiceContext()

Dan.Core/Extensions/ILoggerExtension.cs:53

  • The code uses Path.GetFileNameWithoutExtension but the file does not import System.IO. Add using System.IO; to resolve the reference.
            Path.GetFileNameWithoutExtension(callingClass), callingMethod, null, "", "", owner, requestor, "","", DateTime.UtcNow, serviceContext);      

Comment on lines +96 to +97
ServiceOwner = serviceContexts
.FirstOrDefault(s => s.Name == row["serviceContext"].ToString()).Owner
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

FirstOrDefault may return null if a serviceContext isn't found, causing a NullReferenceException. Consider using a dictionary lookup or null-check before accessing .Owner.

Suggested change
ServiceOwner = serviceContexts
.FirstOrDefault(s => s.Name == row["serviceContext"].ToString()).Owner
ServiceOwner = serviceContexts.TryGetValue(row["serviceContext"].ToString(), out var owner)
? owner
: string.Empty

Copilot uses AI. Check for mistakes.
Comment on lines +96 to +97
ServiceOwner = serviceContexts
.FirstOrDefault(s => s.Name == row["serviceContext"].ToString()).Owner
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

Performing FirstOrDefault inside the loop can be inefficient for large result sets. Consider building a lookup dictionary once before iterating.

Suggested change
ServiceOwner = serviceContexts
.FirstOrDefault(s => s.Name == row["serviceContext"].ToString()).Owner
ServiceOwner = serviceContextLookup.TryGetValue(row["serviceContext"].ToString(), out var owner) ? owner : null

Copilot uses AI. Check for mistakes.
@erlendoksvoll erlendoksvoll merged commit 93bd302 into master Jul 10, 2025
3 checks passed
@erlendoksvoll erlendoksvoll deleted the feature/parquet-statistics branch July 10, 2025 07:00
@sonarqubecloud
Copy link

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.

3 participants