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(taps): Make stream logger a child of the tap logger #1854

Merged
merged 11 commits into from
Nov 14, 2023

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Jul 19, 2023

  • Backwards compatible. Referencing the parent <tap name> in logging config logger will continue work as expected.

Closes #333


📚 Documentation preview 📚: https://meltano-sdk--1854.org.readthedocs.build/en/1854/

@edgarrmondragon edgarrmondragon linked an issue Jul 19, 2023 that may be closed by this pull request
@edgarrmondragon
Copy link
Collaborator Author

edgarrmondragon commented Jul 19, 2023

Example logs:

2023-07-18 18:49:44,885 | INFO     | sample-tap-countries.continents | Beginning full_table sync of 'continents'...
2023-07-18 18:49:44,885 | INFO     | sample-tap-countries.continents | Tap has custom mapper. Using 1 provided map(s).
2023-07-18 18:49:45,487 | INFO     | singer_sdk.metrics   | METRIC: {"type": "timer", "metric": "http_request_duration", "value": 0.521216, "tags": {"stream": "continents", "endpoint": "", "http_status_code": 200, "status": "succeeded"}}
2023-07-18 18:49:45,500 | INFO     | singer_sdk.metrics   | METRIC: {"type": "counter", "metric": "http_request_count", "value": 1, "tags": {"stream": "continents", "endpoint": ""}}
2023-07-18 18:49:45,500 | INFO     | singer_sdk.metrics   | METRIC: {"type": "timer", "metric": "sync_duration", "value": 0.6148860454559326, "tags": {"stream": "continents", "context": {}, "status": "succeeded"}}
2023-07-18 18:49:45,500 | INFO     | singer_sdk.metrics   | METRIC: {"type": "counter", "metric": "record_count", "value": 7, "tags": {"stream": "continents", "context": {}}}
2023-07-18 18:49:45,500 | INFO     | sample-tap-countries.countries | Beginning full_table sync of 'countries'...
2023-07-18 18:49:45,500 | INFO     | sample-tap-countries.countries | Tap has custom mapper. Using 1 provided map(s).
2023-07-18 18:49:45,919 | INFO     | singer_sdk.metrics   | METRIC: {"type": "timer", "metric": "http_request_duration", "value": 0.415213, "tags": {"stream": "countries", "endpoint": "", "http_status_code": 200, "status": "succeeded"}}
2023-07-18 18:49:45,953 | INFO     | singer_sdk.metrics   | METRIC: {"type": "counter", "metric": "http_request_count", "value": 1, "tags": {"stream": "countries", "endpoint": ""}}
2023-07-18 18:49:45,953 | INFO     | singer_sdk.metrics   | METRIC: {"type": "timer", "metric": "sync_duration", "value": 0.4528181552886963, "tags": {"stream": "countries", "context": {}, "status": "succeeded"}}
2023-07-18 18:49:45,953 | INFO     | singer_sdk.metrics   | METRIC: {"type": "counter", "metric": "record_count", "value": 250, "tags": {"stream": "countries", "context": {}}}

Note the sample-tap-countries.continents and sample-tap-countries.countries logger names.

This works together really nice with custom logging:

version: 1
disable_existing_loggers: false
loggers:
  singer_sdk.metrics:
    level: ERROR
  sample-tap-countries.continents:
    level: WARNING
  sample-tap-countries.countries:
    level: DEBUG

Results in

2023-07-19 10:37:52,610 | INFO     | sample-tap-countries.countries | Beginning full_table sync of 'countries'...
2023-07-19 10:37:52,610 | INFO     | sample-tap-countries.countries | Tap has custom mapper. Using 1 provided map(s).
2023-07-19 10:37:52,611 | DEBUG    | sample-tap-countries.countries | Attempting query:
query { 
        countries {
            code
            name
            native
            phone
            continent {
                code
                name
            }
            capital
            currency
            languages {
                code
                name
            }
            emoji
        }
         }

@codecov
Copy link

codecov bot commented Jul 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (466d60a) 86.83% compared to head (7b52e59) 86.83%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1854   +/-   ##
=======================================
  Coverage   86.83%   86.83%           
=======================================
  Files          58       58           
  Lines        4885     4885           
  Branches      777      777           
=======================================
  Hits         4242     4242           
  Misses        456      456           
  Partials      187      187           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edgarrmondragon edgarrmondragon marked this pull request as ready for review July 20, 2023 22:27
@edgarrmondragon edgarrmondragon requested a review from a team July 20, 2023 22:27
@edgarrmondragon edgarrmondragon force-pushed the 333-consider-making-logging-more-granular branch from 7e33b31 to de69d50 Compare September 6, 2023 22:49
@edgarrmondragon edgarrmondragon changed the title feat: Make stream logger a child of the tap logger feat(taps): Make stream logger a child of the tap logger Sep 26, 2023
Copy link

codspeed-hq bot commented Nov 14, 2023

CodSpeed Performance Report

Merging #1854 will not alter performance

Comparing 333-consider-making-logging-more-granular (7b52e59) with main (b83457b)

Summary

✅ 1 untouched benchmarks

@edgarrmondragon edgarrmondragon merged commit d04d39c into main Nov 14, 2023
29 checks passed
@edgarrmondragon edgarrmondragon deleted the 333-consider-making-logging-more-granular branch November 14, 2023 23:40
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.

Consider making logging more granular
1 participant