Feature/audit strategy log #356

andrestejerina97 · 2025-09-29T23:14:33Z

ref: https://app.clickup.com/t/86b6r27rt

…rformance chore: add needed IDX

Copilot

Pull Request Overview

This PR introduces an OpenTelemetry (OTLP) audit strategy for logging entity changes as telemetry data instead of traditional database records. The implementation provides a new audit mechanism that can send audit logs to external observability systems like Elasticsearch through OpenTelemetry collectors.

Implements OTLP-based audit strategy using OpenTelemetry logging
Adds comprehensive test coverage for the new audit functionality
Configures OpenTelemetry collector to export logs to Elasticsearch

Reviewed Changes

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

Show a summary per file

File	Description
app/Audit/Interfaces/IAuditStrategy.php	Defines contract for audit strategy implementations
app/Audit/AuditLogOtlpStrategy.php	Main OTLP audit strategy implementation with telemetry logging
app/Audit/AuditEventListener.php	Updated to support strategy selection between DB and OTLP auditing
tests/OpenTelemetry/AuditOtlpStrategyTest.php	Comprehensive tests for OTLP audit strategy with real entities
tests/OpenTelemetry/AuditEventTypesTest.php	Tests for different audit event types (create, update, delete)
tests/OpenTelemetry/OpenTelemetryTestCase.php	Adds OTEL_LOGS_EXPORTER environment variable for test isolation
docker-compose/opentelemetry/otel-collector-config.yaml	Configures Elasticsearch as logs exporter
.env.example	Adds configuration for Elasticsearch index name

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T15:04:38Z

app/Audit/AuditLogOtlpStrategy.php

+            $data['id'] = $entity->getId();
+        }
+
+        if (method_exists($entity, 'getTitle')) {
+            $data['title'] = $entity->getTitle();
+        }
+
+        if (method_exists($entity, 'getName')) {
+            $data['name'] = $entity->getName();
+        }
+
+        if (method_exists($entity, 'getSlug')) {
+            $data['slug'] = $entity->getSlug();


Consider adding null checks for method return values to prevent null values in audit data, which could cause issues downstream in log processing systems.

Suggested change

$data['id'] = $entity->getId();

}

if (method_exists($entity, 'getTitle')) {

$data['title'] = $entity->getTitle();

}

if (method_exists($entity, 'getName')) {

$data['name'] = $entity->getName();

}

if (method_exists($entity, 'getSlug')) {

$data['slug'] = $entity->getSlug();

$id = $entity->getId();

if ($id !== null) {

$data['id'] = $id;

}

}

if (method_exists($entity, 'getTitle')) {

$title = $entity->getTitle();

if ($title !== null) {

$data['title'] = $title;

}

}

if (method_exists($entity, 'getName')) {

$name = $entity->getName();

if ($name !== null) {

$data['name'] = $name;

}

}

if (method_exists($entity, 'getSlug')) {

$slug = $entity->getSlug();

if ($slug !== null) {

$data['slug'] = $slug;

}

app/Audit/AuditEventListener.php

Copilot · 2025-10-03T15:04:39Z

tests/OpenTelemetry/AuditOtlpStrategyTest.php

+        ];
+    }
+
+    private function createSummitEventChangeSet($summitEvent): array


Parameter should have a type hint for better type safety and IDE support. Consider adding the specific entity type or a base interface.

Suggested change

private function createSummitEventChangeSet($summitEvent): array

private function createSummitEventChangeSet(object $summitEvent): array

romanetar

LGTM

smarcet

@andrestejerina97 please review comments

smarcet · 2025-10-03T21:20:12Z

app/Audit/AuditEventListener.php

-        foreach ($uow->getScheduledCollectionUpdates() as $col) {
-            $strategy->audit($col, null, $strategy::EVENT_COLLECTION_UPDATE);
+        // Check if OTLP audit is enabled
+        if (env('OTEL_SERVICE_ENABLED', false)) {


please review this logic

if (!env('OTEL_SERVICE_ENABLED', false)) return new AuditLogStrategy($em); try { return App::make(AuditLogOtlpStrategy::class); } catch (\Exception $e) { Log::warning('Failed to create OTLP audit strategy, falling back to database', [ 'error' => $e->getMessage() ]); // Fall through to database strategy } return null

I adjusted the logic to use OTLP, and if that fails, it uses AuditLog DB

smarcet

@andrestejerina97 please review comments

Copilot

Pull Request Overview

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

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-03T21:46:05Z

app/Audit/AuditLogOtlpStrategy.php

+        $this->enabled = env('OTEL_SERVICE_ENABLED', false);
+
+        $this->elasticIndex = env('OTEL_AUDIT_ELASTICSEARCH_INDEX', 'logs-audit');


The env() function should not be used directly in classes as it bypasses Laravel's configuration caching. Use config() instead and define the configuration in a config file.

Suggested change

$this->enabled = env('OTEL_SERVICE_ENABLED', false);

$this->elasticIndex = env('OTEL_AUDIT_ELASTICSEARCH_INDEX', 'logs-audit');

$this->enabled = config('opentelemetry.service_enabled', false);

$this->elasticIndex = config('opentelemetry.audit_elasticsearch_index', 'logs-audit');

app/Audit/AuditLogOtlpStrategy.php

app/Audit/AuditEventListener.php

app/Audit/AuditLogOtlpStrategy.php

Copilot

Pull Request Overview

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

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

app/Audit/AuditLogOtlpStrategy.php

app/Audit/AuditEventListener.php

smarcet

LGTM

smarcet force-pushed the main branch 10 times, most recently from e3e142a to 161d4d3 Compare October 1, 2025 02:13

andrestejerina97 force-pushed the feature/audit-strategy-log branch 2 times, most recently from ba4fb8d to c745dbc Compare October 1, 2025 15:32

andrestejerina97 marked this pull request as ready for review October 1, 2025 17:48

andrestejerina97 requested a review from smarcet October 1, 2025 17:49

smarcet force-pushed the main branch 6 times, most recently from 76ff504 to 65b16a8 Compare October 2, 2025 01:04

chore: add to table SummitAttendeeTicket fild summit id to improve pe…

c94fc68

…rformance chore: add needed IDX

smarcet force-pushed the main branch from c94fc68 to 9a8387b Compare October 2, 2025 17:58

smarcet requested review from Copilot and romanetar October 3, 2025 15:03

Copilot AI reviewed Oct 3, 2025

View reviewed changes

romanetar approved these changes Oct 3, 2025

View reviewed changes

Merge branch 'main' of github.com:OpenStackweb/summit-api

8762fef

smarcet requested changes Oct 3, 2025

View reviewed changes

smarcet reviewed Oct 3, 2025

View reviewed changes

smarcet requested changes Oct 3, 2025

View reviewed changes

andrestejerina97 added 2 commits October 3, 2025 18:35

feat: add log audit strategy for replace audit in BBDD

7ad4b4c

feat: Add unit test for Audit Otlp Strategy

5b430dd

andrestejerina97 force-pushed the feature/audit-strategy-log branch from d982d42 to 06ab148 Compare October 3, 2025 21:42

andrestejerina97 requested a review from Copilot October 3, 2025 21:45

Copilot AI reviewed Oct 3, 2025

View reviewed changes

andrestejerina97 added 2 commits October 3, 2025 18:49

fix: add null checks to EventListener, fix in getAuditStrategy

de4d063

fix: change env function by config function

9c4bf52

andrestejerina97 force-pushed the feature/audit-strategy-log branch from 06ab148 to 9c4bf52 Compare October 3, 2025 22:29

andrestejerina97 requested a review from Copilot October 3, 2025 22:32

Copilot AI reviewed Oct 3, 2025

View reviewed changes

app/Audit/AuditLogOtlpStrategy.php Outdated Show resolved Hide resolved

app/Audit/AuditLogOtlpStrategy.php Show resolved Hide resolved

app/Audit/AuditEventListener.php Outdated Show resolved Hide resolved

fix: add improvements in the use of constants, remove code unnecessary

2072284

andrestejerina97 requested a review from smarcet October 3, 2025 23:59

smarcet approved these changes Oct 6, 2025

View reviewed changes

smarcet merged commit 48b51f8 into main Oct 6, 2025
3 checks passed

-            $data['id'] = $entity->getId();
-        }
-        if (method_exists($entity, 'getTitle')) {
-            $data['title'] = $entity->getTitle();
-        }
-        if (method_exists($entity, 'getName')) {
-            $data['name'] = $entity->getName();
-        }
-        if (method_exists($entity, 'getSlug')) {
-            $data['slug'] = $entity->getSlug();
+            $id = $entity->getId();
+            if ($id !== null) {
+                $data['id'] = $id;
+            }
+        }
+        if (method_exists($entity, 'getTitle')) {
+            $title = $entity->getTitle();
+            if ($title !== null) {
+                $data['title'] = $title;
+            }
+        }
+        if (method_exists($entity, 'getName')) {
+            $name = $entity->getName();
+            if ($name !== null) {
+                $data['name'] = $name;
+            }
+        }
+        if (method_exists($entity, 'getSlug')) {
+            $slug = $entity->getSlug();
+            if ($slug !== null) {
+                $data['slug'] = $slug;
+            }

	private function createSummitEventChangeSet($summitEvent): array
	private function createSummitEventChangeSet(object $summitEvent): array

		$this->enabled = env('OTEL_SERVICE_ENABLED', false);

		$this->elasticIndex = env('OTEL_AUDIT_ELASTICSEARCH_INDEX', 'logs-audit');

Feature/audit strategy log #356

Feature/audit strategy log #356

Conversation

andrestejerina97 commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

andrestejerina97 Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

romanetar left a comment

Choose a reason for hiding this comment

Uh oh!

smarcet left a comment

Choose a reason for hiding this comment

Uh oh!

smarcet Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

andrestejerina97 Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

smarcet left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarcet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrestejerina97 commented Sep 29, 2025 •

edited

Loading