Skip to content

feat(identity): improve identity migration logging verbosity and reduce noise#1050

Draft
Copilot wants to merge 4 commits intomainfrom
copilot/improve-logging-verbosity
Draft

feat(identity): improve identity migration logging verbosity and reduce noise#1050
Copilot wants to merge 4 commits intomainfrom
copilot/improve-logging-verbosity

Conversation

Copy link
Contributor

Copilot AI commented Feb 27, 2026

Authorization and tenant migration logs were hard to follow—skipped messages only showed UUIDs, making it impossible to identify the actual entities without a database lookup. Additionally, common 409 conflicts (authorization/tenant already exists) were logged as ERROR with full stack traces, polluting logs with noise for an expected scenario.

Description

Verbose authorization logs (IdentityMigratorLogs, IdentityMigrator)

All authorization log messages now include owner type/ID, resource type, and resource ID alongside the authorization UUID.

Before:

WARN: Authorization with ID [dc09b223-125c-11f1-bc85-d251a5285969] was skipped: Permission type [UPDATE] is not supported for resource type [DecisionDefinition]

After:

WARN: Authorization with ID [dc09b223-...] for user [jonny1] on DecisionDefinition [my-process] was skipped: Permission type [UPDATE] is not supported for resource type [DecisionDefinition]
  • logMigratingAuthorization, logMigratedAuthorization, logSkippedAuthorization now accept an Authorization object
  • Added markAsSkipped(Authorization, String) overload to IdentityMigrator to carry the full authorization context through to the log call

Verbose tenant logs (IdentityMigratorLogs, IdentityMigrator)

All tenant log messages now include the tenant name alongside the tenant ID. The skip reason is also now logged for skipped tenants (it was previously silently dropped — a pre-existing bug fixed here).

Before:

WARN: Tenant with ID [tenantId2-!~^] was skipped

After:

WARN: Tenant with ID [tenantId2-!~^] (name: tenantName2) was skipped: Tenant identifier must match pattern...
  • logMigratingTenant, logMigratedTenant, logSkippedTenant now accept a Tenant object
  • Added markAsSkipped(Tenant, String) overload to IdentityMigrator
  • Removed now-unused markAsSkipped(IdKeyMapper.TYPE, String, String) dead code

Suppress 409 stack traces (ExceptionUtils)

In IDENTITY context, a ProblemException with HTTP 409 is now logged as WARN with the API detail message only—no stack trace. All other errors retain existing ERROR+stack-trace behavior.

Before:

ERROR: Failed to migrate authorization with legacy ID: bcae2d10-...
io.camunda.migration.data.exception.IdentityMigratorException: ...
  at ... [40+ lines of stack trace]
Caused by: io.camunda.client.api.command.ProblemException: 409 ALREADY_EXISTS ...

After:

WARN: Failed to migrate authorization with legacy ID: bcae2d10-...: Command 'CREATE' rejected with code 'ALREADY_EXISTS': Expected to create authorization for owner 'jonny1'...

Test updates (IdentityTestHelper)

verifyAuthorizationSkippedViaLogs and verifyTenantSkippedViaLogs updated from brittle fixed-format string matches to anySatisfy assertions that verify the entity ID and skip reason appear together in the same log event.

Architecture compliance (IdentityMigratorLogs)

Helper methods ownerType, ownerId, and resourceTypeName changed from private to package-private visibility to satisfy the project's ArchitectureTest rule requiring methods in io.camunda.migration.data.. packages to use protected or package-private access.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Test-only changes (no production code changes)

Testing Checklist

Black-Box Testing Requirements

  • Tests follow black-box testing approach: verify behavior through observable outputs (logs, C8 API queries, real-world skip scenarios)
  • Tests DO NOT access implementation details (DbClient, IdKeyMapper, ..impl.. packages except logging constants)
  • Architecture tests pass (ArchitectureTest validates these rules)

Test Coverage

  • Added tests for new functionality
  • Updated tests for modified functionality
  • All tests pass locally

Architecture Compliance

Run architecture tests to ensure compliance:

mvn test -Dtest=ArchitectureTest

If architecture tests fail, refactor your tests to use:

  • LogCapturer for log assertions
  • camundaClient.new*SearchRequest() for C8 queries
  • Real-World skip scenarios (e.g., migrate children without parents)

Documentation

  • Updated TESTING_GUIDELINES.md if adding new test patterns
  • Added Javadoc comments for public APIs
  • Updated README if user-facing changes

Checklist

  • Code follows project style guidelines
  • Self-reviewed the code
  • Added comments for complex logic
  • No new compiler warnings
  • Dependent changes have been merged

Related Issues

Related to #1011

Original prompt

This section details on the original issue you should resolve

<issue_title>Identity Migration: Improve logging</issue_title>
<issue_description>### Acceptance Criteria (Required on creation)

Details in original thread.

  1. Make logs, in general, more verbose and log user/group, permissions, and resource IDs? The authorization ID is a technical detail that users will have difficulty following up on.
  2. Avoid logging exception stack traces for common cases like 'authorization already exists'.

Hints

Links

Breakdown

Pull Requests

Dev2QA handover

  • Does this ticket need a QA test, and are the testing goals not clear from the description? Add a Dev2QA handover comment
    </issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Add user/group, resource type, and resource ID to authorization log messages
- Suppress stack traces for 409 "already exists" conflicts in identity context
- Update test helper to match new log format using anySatisfy assertion

related to #1011

Co-authored-by: tasso94 <[email protected]>
Copilot AI changed the title [WIP] Improve logging for identity migration feat(identity): improve authorization migration logging verbosity and reduce noise Feb 27, 2026
@tasso94
Copy link
Member

tasso94 commented Feb 27, 2026

@copilot, would a similar improvement also make sense for tenants/tenant memberships?

- Include tenant name in MIGRATING_TENANT, SUCCESSFULLY_MIGRATED_TENANT, SKIPPED_TENANT messages
- Include skip reason in SKIPPED_TENANT (was previously silently dropped)
- Update logMigratingTenant/logMigratedTenant/logSkippedTenant to accept Tenant object
- Add markAsSkipped(Tenant, String) overload in IdentityMigrator
- Remove now-unused markAsSkipped(IdKeyMapper.TYPE, String, String) dead code
- Update verifyTenantSkippedViaLogs to use anySatisfy pattern

related to #1011

Co-authored-by: tasso94 <[email protected]>
Copy link
Contributor Author

Copilot AI commented Feb 27, 2026

@copilot, would a similar improvement also make sense for tenants/tenant memberships?

Yes, applied the same improvements in commit 558fc64:

  • Tenant log messages now include the tenant name alongside the ID in all messages
  • SKIPPED_TENANT now also includes the skip reason (it was previously being passed in but silently dropped — a pre-existing bug fixed as part of this)

Example new format:

  • "Successfully migrated tenant [tenantId1] (name: tenantName1)"
  • "Tenant with ID [tenantId2-!~^] (name: tenantName2) was skipped: Tenant identifier must match pattern..."

Tenant membership logs were already verbose (include tenant ID, member type, and user/group ID), so no changes were needed there.

Copilot AI changed the title feat(identity): improve authorization migration logging verbosity and reduce noise feat(identity): improve identity migration logging verbosity and reduce noise Feb 27, 2026
@tasso94
Copy link
Member

tasso94 commented Feb 27, 2026

@copilot, fix test failures:

Error:  Failures: 
Error:    ArchitectureTest.shouldNotHavePrivateMethods:77 Architecture Violation [Priority: MEDIUM] - Rule 'classes that reside in a package 'io.camunda.migration.data..' and reside outside of package '..qa..' should not have private methods, because Methods should use protected or package-protected visibility instead of private to allow for testing and extensibility. Use protected for methods that might be overridden and package-protected for internal methods that tests might need to access.' was violated (3 times):
Method <io.camunda.migration.data.impl.logging.IdentityMigratorLogs.ownerId()> is private, should be protected or package-protected
Method <io.camunda.migration.data.impl.logging.IdentityMigratorLogs.ownerType()> is private, should be protected or package-protected
Method <io.camunda.migration.data.impl.logging.IdentityMigratorLogs.resourceTypeName()> is private, should be protected or package-protected

…entityMigratorLogs

ArchitectureTest requires methods in io.camunda.migration.data.. packages to use
protected or package-private visibility instead of private.

related to #1011

Co-authored-by: tasso94 <[email protected]>
Copy link
Contributor Author

Copilot AI commented Feb 27, 2026

@copilot, fix test failures:

Error:  Failures: 

...

Fixed in commit 5ad4ab5 — changed the 3 private static helper methods (ownerType, ownerId, resourceTypeName) in IdentityMigratorLogs to package-private (static without modifier) to satisfy the ArchitectureTest rule.

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.

Identity Migration: Improve logging

2 participants