Skip to content

[#noissue] Dump the full application map model when debug logging is enabled - #14152

Open
emeroad wants to merge 2 commits into
pinpoint-apm:masterfrom
emeroad:mapmodel
Open

[#noissue] Dump the full application map model when debug logging is enabled#14152
emeroad wants to merge 2 commits into
pinpoint-apm:masterfrom
emeroad:mapmodel

Conversation

@emeroad

@emeroad emeroad commented Aug 14, 2026

Copy link
Copy Markdown
Member

This pull request refactors the application map building and writing logic to improve clarity and efficiency in the Pinpoint collector. The changes focus on replacing the isEmpty() method with a more intuitive hasRows() method, enhancing the dump() output for debugging, and updating dependency injection for ApplicationMapBuilder. These updates help ensure that only non-empty models are processed and logged, and improve code maintainability.

Refactoring and API improvements:

  • Replaced isEmpty() with hasRows() in ApplicationMapModel, inverting the logic to clarify when the model contains data. All usages and tests have been updated accordingly. [1] [2]
  • Enhanced the dump() method in ApplicationMapModel to produce a more readable, indented, multi-line output, and omit empty lists for easier debugging.

Dependency injection and service construction:

  • Refactored dependency injection in ApplicationMapModule and HbaseApplicationMapService to inject an ApplicationMapBuilder instance directly, rather than constructing it internally, improving testability and separation of concerns. [1] [2] [3] [4]

Behavioral changes:

  • Updated the write() method in HbaseApplicationMapService to only log and process models that have rows, preventing unnecessary operations on empty models. [1] [2]

@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the collector’s application map write path to better distinguish empty vs non-empty ApplicationMapModel instances, improves debug dump readability, and makes ApplicationMapBuilder injectable for cleaner construction and testing.

Changes:

  • Replaces ApplicationMapModel.isEmpty() with hasRows() (inverted semantics) and updates call sites/tests.
  • Enhances ApplicationMapModel.dump() to emit an indented multi-line format and omit empty lists.
  • Refactors HbaseApplicationMapService / ApplicationMapModule to inject ApplicationMapBuilder instead of constructing it internally, and skips write/log work for empty models.

Reviewed changes

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

File Description
collector/src/test/java/com/navercorp/pinpoint/collector/applicationmap/model/ApplicationMapBuilderTest.java Updates expectations to use hasRows() for empty model assertions.
collector/src/main/java/com/navercorp/pinpoint/collector/applicationmap/service/HbaseApplicationMapService.java Injects ApplicationMapBuilder and gates debug logging + persistence operations on model.hasRows().
collector/src/main/java/com/navercorp/pinpoint/collector/applicationmap/model/ApplicationMapModel.java Introduces hasRows() and upgrades dump() output formatting/verbosity for debugging.
collector/src/main/java/com/navercorp/pinpoint/collector/applicationmap/config/ApplicationMapModule.java Adds an ApplicationMapBuilder bean and wires it into HbaseApplicationMapService.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants