[Bug]: Metric logging is not spec compliant

[Jack-Burnett]

Singer SDK Version

0.6.1

Python Version

3.9

Bug scope

Taps (catalog, state, stream maps, etc.)

Operating System

MacOS

Description

A metric log looks like this:
time=2022-08-01 16:19:29 name=tap-tabular-sftp level=INFO message=INFO METRIC: {'type': 'counter', 'metric': 'record_count', 'value': 0, 'tags': {'stream': 'test'}}

The issue is that it uses ' instead of " so is not valid JSON that other tools would be able to parse.

I think this line:

sdk/singer_sdk/streams/core.py

Line 820 in 4762cfd

self._metric_logging_function(f"INFO METRIC: {str(metric)}")

would simple need to be changed to a json dump instead of directly trying to log the object.

As is, it is not compliant with the spec at https://hub.meltano.com/singer/spec#metrics

Code

No response

[edgarrmondragon]

@Jack-Burnett thanks for filing! I agree. The issue is str is being used instead of json.dumps to cast the metric.

It's an easy fix. Would you have the time and be willing to submit a PR?

[Jack-Burnett]

Sure thing

[Jack-Burnett]

Sorry if this is a silly question, but do I need to do something to be able to make a PR?
After creating a branch and then attempting to run;
git push --set-upstream origin fix_metric_format
I get back
remote: Permission to meltano/sdk.git denied to Jack-Burnett. fatal: unable to access 'https://github.com/meltano/sdk.git/': The requested URL returned error: 403

[edgarrmondragon]

@Jack-Burnett Ah, you need to fork this repository, commit the changes to the patch branch in your personal fork, and then submit a PR from your fork to the main branch in this repo. Let me know if I can help with any step of the process 🙂.