refactor: logging enhancements around seeding#440
Open
catastrophe-brandon wants to merge 1 commit intomainfrom
Open
refactor: logging enhancements around seeding#440catastrophe-brandon wants to merge 1 commit intomainfrom
catastrophe-brandon wants to merge 1 commit intomainfrom
Conversation
Contributor
Reviewer's GuideRefactors the database seeding flow to use structured logrus logging throughout, adding detailed phase, progress, per-entity, and summary logs while improving error handling and returning early on failures in the seeding helpers. Sequence diagram for SeedTags orchestration with logging and error handlingsequenceDiagram
participant Seeder as SeedTags
participant Logger as logrus
participant DB as DB
participant Clear as clearOldContent
participant SeedTagsHelper as seedDefaultTags
participant Find as findTags
participant SeedQS as seedQuickstart
participant SeedHT as seedHelpTopic
participant SeedFav as SeedFavorites
Seeder->>Logger: Info Starting database seeding process...
Seeder->>Clear: clearOldContent()
Clear->>Logger: Info Clearing old content...
Clear->>DB: Load and delete stale favorites, quickstarts, help topics, tags
Clear->>Logger: Infof Cleared favorites, quickstarts, help topics, tags counts
Clear-->>Seeder: favorites
Seeder->>SeedTagsHelper: seedDefaultTags()
SeedTagsHelper->>Logger: Info Seeding default tags...
SeedTagsHelper->>DB: FirstOrCreate quickstartsKindTag
alt quickstartsKindTag error
SeedTagsHelper->>Logger: Errorf Unable to create quickstarts kind tag
end
SeedTagsHelper->>DB: FirstOrCreate helpTopicKindTag
alt helpTopicKindTag error
SeedTagsHelper->>Logger: Errorf Unable to create help topic kind tag
end
SeedTagsHelper->>Logger: Info Default tags seeded successfully
SeedTagsHelper-->>Seeder: defaultTags map
Seeder->>Find: findTags()
Find->>DB: Glob quickstarts metadata files
alt glob quickstarts error
Find->>Logger: Fatalf Failed to find quickstarts metadata files
end
Find->>DB: Glob help topics metadata files
alt glob help topics error
Find->>Logger: Fatalf Failed to find help topics metadata files
end
Find->>Logger: Infof Found metadata files and counts
loop for each metadata file
Find->>Find: readMetadata(file)
alt readMetadata error
Find->>Logger: Warnf Failed to read metadata from file
else success
Find->>Find: append to MetadataTemplates
end
end
Find->>Logger: Infof Successfully parsed metadata templates
Find-->>Seeder: MetadataTemplates
Seeder->>Seeder: Initialize quickstartCount, quickstartErrorCount, helpTopicCount, helpTopicErrorCount
Seeder->>Logger: Infof Processing templates count
loop for each MetadataTemplate
alt template.Kind == QuickStarts
Seeder->>SeedQS: seedQuickstart(template, defaultTags quickstart)
SeedQS->>Seeder: addTags(template)
alt addTags error
SeedQS->>Logger: Errorf Failed to add tags for quickstart
SeedQS-->>Seeder: error
else addTags ok
SeedQS->>DB: Where name, find existing quickstart
alt DB error
SeedQS->>Logger: Errorf Database error while checking for existing quickstart
SeedQS-->>Seeder: error
else no existing quickstart
SeedQS->>DB: Create quickstart
SeedQS->>DB: Associate quickstart with defaultTag
alt association error
SeedQS->>Logger: Errorf Failed creating quickstarts default tag associations
end
SeedQS->>Logger: Infof Created new quickstart
SeedQS-->>Seeder: quickstart
else existing quickstart
SeedQS->>DB: Clear Tags association
alt clear error
SeedQS->>Logger: Errorf Failed clearing tags associations for quickstart
end
SeedQS->>DB: Associate defaultTag with quickstart
alt association error
SeedQS->>Logger: Errorf Failed creating quickstarts default tag associations
end
SeedQS->>Logger: Infof Updated existing quickstart
SeedQS-->>Seeder: updated quickstart
end
end
alt seedQuickstart error
Seeder->>Logger: Errorf Unable to seed quickstart from template
Seeder->>Seeder: Increment quickstartErrorCount
else seedQuickstart ok
Seeder->>Seeder: Increment quickstartCount
loop for each tag in template.Tags
Seeder->>DB: Preload Quickstarts and find or create tag
alt DB error
Seeder->>Logger: Errorf Database error while finding tag
end
Seeder->>DB: Associate quickstart with tag
alt association error
Seeder->>Logger: Errorf Failed creating tag association for quickstart
end
end
end
else template.Kind == HelpTopic
Seeder->>SeedHT: seedHelpTopic(template, defaultTags helptopic)
SeedHT->>Logger: Errorf on file read, YAML to JSON, JSON unmarshal, DB errors, marshal errors, tag clear or association failures as they occur
SeedHT-->>Seeder: helpTopics or error
alt seedHelpTopic error
Seeder->>Logger: Errorf Unable to seed help topic from template
Seeder->>Seeder: Increment helpTopicErrorCount
else seedHelpTopic ok
Seeder->>Seeder: Increment helpTopicCount by number of help topics
loop for each tag in template.Tags
Seeder->>DB: Preload HelpTopics and find or create tag
alt DB error
Seeder->>Logger: Errorf Database error while finding tag
end
Seeder->>DB: Clear HelpTopics association on tag
alt clear error
Seeder->>Logger: Errorf Failed clearing help topic tag associations
end
Seeder->>DB: Associate help topics with tag
alt association error
Seeder->>Logger: Errorf Failed creating tag association for help topics
end
end
end
else other kind
Seeder->>Seeder: Skip template
end
end
Seeder->>Logger: Infof Content seeding summary quickstarts and help topics counts and errors
Seeder->>SeedFav: SeedFavorites(favorites)
SeedFav->>DB: Seed favorites using favorite quickstarts
Seeder->>Logger: Info Database seeding completed
Seeder-->>Seeder: End SeedTags
Flow diagram for enhanced SeedTags seeding process with loggingflowchart TD
SeedTagsStart([SeedTags entrypoint])
SeedTagsStart --> LogStart["logrus.Info Starting database seeding process..."]
LogStart --> ClearOldContent["clearOldContent"]
ClearOldContent --> LogClearPhase["logrus.Info Clearing old content..."]
LogClearPhase --> DeleteOldData["Delete stale favorites, quickstarts, help topics, tags"]
DeleteOldData --> LogClearSummary["logrus.Infof Cleared favorites, quickstarts, help topics, tags counts"]
LogClearSummary --> ReturnFavorites["Return favorites slice"]
ReturnFavorites --> SeedDefaultTags["seedDefaultTags"]
SeedDefaultTags --> LogSeedTagsStart["logrus.Info Seeding default tags..."]
LogSeedTagsStart --> CreateOrFindKindTags["Create or find quickstart and help topic kind tags"]
CreateOrFindKindTags --> LogKindTagErrors["logrus.Errorf on failure to create kind tags"]
CreateOrFindKindTags --> LogSeedTagsDone["logrus.Info Default tags seeded successfully"]
LogSeedTagsDone --> FindTags["findTags"]
FindTags --> GlobQuickstarts["Glob quickstarts metadata files"]
GlobQuickstarts --> GlobQuickstartsError{Glob quickstarts error?}
GlobQuickstartsError -- yes --> LogQuickstartsFatal["logrus.Fatalf Failed to find quickstarts metadata files"]
GlobQuickstartsError -- no --> GlobHelpTopics["Glob help topics metadata files"]
GlobHelpTopics --> GlobHelpTopicsError{Glob help topics error?}
GlobHelpTopicsError -- yes --> LogHelpTopicsFatal["logrus.Fatalf Failed to find help topics metadata files"]
GlobHelpTopicsError -- no --> CombineFiles["Combine quickstarts and help topics files"]
CombineFiles --> LogFoundFiles["logrus.Infof Found metadata files and counts"]
LogFoundFiles --> ParseMetadataTemplates["readMetadata for each file"]
ParseMetadataTemplates --> LogReadError["logrus.Warnf on metadata read failure"]
ParseMetadataTemplates --> AccumulateTemplates["Append successful metadata to MetadataTemplates"]
AccumulateTemplates --> LogParsedTemplates["logrus.Infof Successfully parsed metadata templates"]
LogParsedTemplates --> InitCounters["Initialize quickstart and help topic success and error counters to zero"]
InitCounters --> LogProcessingTemplates["logrus.Infof Processing templates count"]
LogProcessingTemplates --> ForEachTemplate{{For each MetadataTemplate}}
ForEachTemplate --> KindQuickStarts{template.Kind == QuickStarts?}
KindQuickStarts -- yes --> SeedQuickstart["seedQuickstart"]
KindQuickStarts -- no --> KindHelpTopic{template.Kind == HelpTopic?}
KindHelpTopic -- yes --> SeedHelpTopic["seedHelpTopic"]
KindHelpTopic -- no --> NextTemplate["Skip template"]
SeedQuickstart --> SeedQuickstartError{Error?}
SeedQuickstartError -- yes --> LogQuickstartSeedError["logrus.Errorf Unable to seed quickstart from template"]
LogQuickstartSeedError --> IncQuickstartError["Increment quickstartErrorCount"]
IncQuickstartError --> AssociateQuickstartTags["Associate tags with quickstart with DB associations and error logs"]
SeedQuickstartError -- no --> IncQuickstartSuccess["Increment quickstartCount"]
IncQuickstartSuccess --> AssociateQuickstartTags
SeedHelpTopic --> SeedHelpTopicError{Error?}
SeedHelpTopicError -- yes --> LogHelpTopicSeedError["logrus.Errorf Unable to seed help topic from template"]
LogHelpTopicSeedError --> IncHelpTopicError["Increment helpTopicErrorCount"]
IncHelpTopicError --> AssociateHelpTopicTags["Associate tags with help topics with DB associations and error logs"]
SeedHelpTopicError -- no --> IncHelpTopicSuccess["Increment helpTopicCount by number of help topics"]
IncHelpTopicSuccess --> AssociateHelpTopicTags
AssociateQuickstartTags --> NextTemplate
AssociateHelpTopicTags --> NextTemplate
NextTemplate --> ForEachTemplate
ForEachTemplate --> AllTemplatesDone["All templates processed"]
AllTemplatesDone --> LogContentSummary["logrus.Infof Content seeding summary with quickstart and help topic counts and errors"]
LogContentSummary --> SeedFavoritesCall["SeedFavorites with favorites from clearOldContent"]
SeedFavoritesCall --> LogCompleted["logrus.Info Database seeding completed"]
LogCompleted --> SeedTagsEnd([SeedTags exit])
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new use of
logrus.FatalfinfindTagswill terminate the whole process on glob failures; consider returning an error up the call stack instead so callers can decide how to handle missing/invalid metadata directories. - Per-resource
Infoflogs inside tight loops (e.g., each quickstart/help topic create/update) may become very noisy at scale; consider downgrading these toDebugor adding a higher-level summary-only mode.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new use of `logrus.Fatalf` in `findTags` will terminate the whole process on glob failures; consider returning an error up the call stack instead so callers can decide how to handle missing/invalid metadata directories.
- Per-resource `Infof` logs inside tight loops (e.g., each quickstart/help topic create/update) may become very noisy at scale; consider downgrading these to `Debug` or adding a higher-level summary-only mode.
## Individual Comments
### Comment 1
<location path="pkg/database/db_seed.go" line_range="336" />
<code_context>
var tags []models.Tag
quickstart, quickstartErr = seedQuickstart(template, defaultTags["quickstart"])
if quickstartErr != nil {
- fmt.Println("Unable to seed quickstart: ", quickstartErr.Error(), template.ContentPath)
+ logrus.Errorf("Unable to seed quickstart from %s: %v", template.ContentPath, quickstartErr)
</code_context>
<issue_to_address>
**issue (bug_risk):** On quickstart seed error we still mutate and save `quickstart`, which is likely a zero-value/invalid record.
If `seedQuickstart` fails, `quickstart` is likely the zero value, but tags are still cleared and `DB.Save(&quickstart)` is called. This can result in inserting/updating an invalid quickstart and obscures the original failure.
Return early when `quickstartErr != nil` (after logging and incrementing `quickstartErrorCount`), or otherwise ensure tag clearing and `DB.Save` only run on success.
</issue_to_address>
### Comment 2
<location path="pkg/database/db_seed.go" line_range="374-382" />
<code_context>
if kind == "HelpTopic" {
helpTopic, helpTopicErr := seedHelpTopic(template, defaultTags["helptopic"])
if helpTopicErr != nil {
- fmt.Println("Unable to seed help topic: ", helpTopicErr.Error(), template.ContentPath)
+ logrus.Errorf("Unable to seed help topic from %s: %v", template.ContentPath, helpTopicErr)
+ helpTopicErrorCount++
+ } else {
+ helpTopicCount += len(helpTopic)
}
for _, tag := range template.Tags {
</code_context>
<issue_to_address>
**issue (bug_risk):** Tag processing for help topics continues even when `seedHelpTopic` returns an error.
On error, we log and increment `helpTopicErrorCount` but still iterate over `template.Tags` and use `helpTopic` for associations. In this case `helpTopic` will be empty (or partially populated), so tag associations become misleading/no-ops. Consider `continue`-ing the outer loop (or otherwise skipping the tag-association block) when `helpTopicErr != nil`, so tags are only associated with successfully seeded help topics.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
@sourcery-ai resolve |
catastrophe-brandon
requested review from
a team,
OlhaTmlk,
charlesmulder,
justinorringer and
karelhala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhanced Logging Output
Issue link: https://redhat.atlassian.net/browse/RHCLOUD-41972
The seeding process now provides clear diagnostic information:
- "Starting database seeding process..."
- "Clearing old content..."
- "Seeding default tags..."
- "Processing templates..."
- Found 109 metadata files (99 quickstarts, 10 help topics)
- Successfully parsed 109 metadata templates
- Individual creation/update messages for each quickstart and help topic
- Cleared 0 favorites, 104 quickstarts, 15 help topics, and 84 tags
- Content seeding summary: 99 quickstarts (0 errors), 18 help topics (0 errors)
- Favorites seeding summary with success/error counts
- "Database seeding completed"
- "Seeding complete"
Summary by Sourcery
Improve observability and robustness of the database seeding process by adding structured logging, detailed progress reporting, and error handling across tag, quickstart, help topic, and favorites seeding.
New Features:
Enhancements: