Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server): add path to metadata logging #16212

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Conversation

etnoy
Copy link
Contributor

@etnoy etnoy commented Feb 19, 2025

This PR adds path information to most log statements made by the metadata service. This makes it easier to see which files have broken metadata, especially with external libraries.

Example:

[Microservices:MetadataService] No timezone information found for asset 1072ebc8-8121-4d43-beed-508473cbd2c4: /data/import/bilder/2021/2021-06/2021-06-17/Original/20210617-064140--IMG_0542.PNG
[Microservices:MetadataService] Date and time is 2021-11-12T10:52:29+01:00 for asset 7b61ee91-2847-4ac8-a28c-2667003d10fa: /data/import/bilder/2021/2021-11/2021-11-12/Original/20211112-105229-SM-G973F-20211112_105229.JPG
[Microservices:MetadataService] Found timezone UTC+1 via OffsetTime for asset 7b61ee91-2847-4ac8-a28c-2667003d10fa: /data/import/bilder/2021/2021-11/2021-11-12/Original/20211112-105229-SM-G973F-20211112_105229.JPG
[Microservices:MetadataService] Found local date time 2021-11-12T10:52:29.000Z for asset 7b61ee91-2847-4ac8-a28c-2667003d10fa: /data/import/bilder/2021/2021-11/2021-11-12/Original/20211112-105229-SM-G973F-20211112_105229.JPG

I've also reworked and clarified some log statements

Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

I disagree. With an id they can quickly go to /photos/:id and check the asset out. With the path they'd need to dig in the folder structure, which is especially annoying if you're not using the storage template or an external library.

@mmomjian
Copy link
Contributor

I personally would prefer that we log both? It’s a lot easier to work with the database or view the asset on the web view using the ID

@etnoy
Copy link
Contributor Author

etnoy commented Feb 19, 2025

Fair points. I first thought of logging path for external assets and id for others, but I didn't want to make exceptions like that. I'll let you guys decide what to do here

@mertalev
Copy link
Contributor

I prefer logging the ID and don't think we need to log the original path as well, which is quite verbose.

@alextran1502
Copy link
Contributor

I am fine with logging both id and path for the sake of traceability

@etnoy etnoy force-pushed the feat/asset-path-in-log branch 2 times, most recently from e16a551 to 55a5729 Compare February 19, 2025 22:12
@etnoy
Copy link
Contributor Author

etnoy commented Feb 19, 2025

I am fine with logging both id and path for the sake of traceability

I've updated the PR with your suggestion, please see the original PR description for an example

@etnoy etnoy changed the title feat(server): Prefer original path instead of id when logging in metadata service feat(server): add path to metadata logging Feb 19, 2025
@etnoy etnoy force-pushed the feat/asset-path-in-log branch 3 times, most recently from cc9b7af to cf2158a Compare February 20, 2025 11:24
@etnoy etnoy force-pushed the feat/asset-path-in-log branch from cf2158a to f562c73 Compare February 20, 2025 11:26
@alextran1502 alextran1502 merged commit f6ba071 into main Feb 20, 2025
39 checks passed
@alextran1502 alextran1502 deleted the feat/asset-path-in-log branch February 20, 2025 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants