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

Report FSL conversion warnings to usage API #68

Merged
merged 8 commits into from
May 28, 2024
Merged

Report FSL conversion warnings to usage API #68

merged 8 commits into from
May 28, 2024

Conversation

nyalldawson
Copy link
Collaborator

No description provided.

@nyalldawson
Copy link
Collaborator Author

@ChrisLoer Would you mind testing this, and seeing if the reporting of FSL conversion warnings to your API is working well?

@ChrisLoer
Copy link
Contributor

I haven't been able to trigger any warnings using the plugin built from this PR (felt_plugin.2.0.2-alpha.ec1d150.zip) using the data/style from: geology_style (1).zip. I tried adding a scale-based rule because I saw that in the list of error messages, but still no dice:

Screenshot 2024-05-27 at 4 37 43 PM

Can you provide some example data/style I could use to trigger the warning?

@nyalldawson
Copy link
Collaborator Author

@ChrisLoer there's no user facing errors yet, but you should be getting reports via the usage API. Can you check that?

@ChrisLoer
Copy link
Contributor

No I'm not seeing anything come in. Glancing at this code, it looks like it's still using unsupported_layer (which is meant to be "we can't upload the data") vs "unsupported_style" (which is supposed to be "we can't automatically style this"). But even looking for unsupported_layer in our logs, I only see reports coming in from the existing 2.0.1 version of the plugin, and nothing so far today.

Is there a way to see the log warning messages from the QGIS side so I can tell what it's trying to do?

@nyalldawson
Copy link
Collaborator Author

Is there a way to see the log warning messages from the QGIS side so I can tell what it's trying to do?

Can you check the network logger requests? For my testing I'm seeing a submission like this:

curl 'https://felt.com/api/v1/internal/reports' -H 'accept: application/json' -H 'x-qgis-add-to-felt-version: 1.0.0' -H 'authorization: Bearer xxxx' -H 'Content-Type: application/json' -H 'User-Agent: Mozilla/5.0 QGIS/33700/Fedora Linux 40 (KDE Plasma)' --data '{"type": "info", "content": "{"type": "fsl_conversion", "warnings": [{"object": "renderer", "renderer": "rule_based", "cause": "scale_based_rule", "message": "Rule based renderer with scale based rule visibility cannot be converted", "level": "Error"}]}"}' --compressed

@ChrisLoer
Copy link
Contributor

Ah yeah, I see it now -- it's coming in with the fsl_conversion type. Sorry I'm badly in need of a nap today. So it's making it to our logs, but not to our upstream analytics because the only two types we automatically record are unsupported_layer and the new unsupported_style using the same item->count syntax that unsupported_layer uses.

Screenshot 2024-05-27 at 5 53 43 PM

1 similar comment
@ChrisLoer
Copy link
Contributor

Ah yeah, I see it now -- it's coming in with the fsl_conversion type. Sorry I'm badly in need of a nap today. So it's making it to our logs, but not to our upstream analytics because the only two types we automatically record are unsupported_layer and the new unsupported_style using the same item->count syntax that unsupported_layer uses.

Screenshot 2024-05-27 at 5 53 43 PM

@nyalldawson
Copy link
Collaborator Author

@ChrisLoer so is that something that needs to change in the way the plugin reports this? or in the analytics backend?

@ChrisLoer
Copy link
Contributor

I think the plugin? This was my original request here https://github.com/felt/qgis-plugin-discussion/issues/71 -- I mostly just chose that format because it's what we were already using for unsupported layers and led to a nice analogous hook up to our analytics. As long as it reports the keys/counts in that format, any other data that gets reported will still be accessible to use for a limited time via the logs (we hold them for two weeks).

@nyalldawson
Copy link
Collaborator Author

@ChrisLoer can you try with the latest version?

@nyalldawson nyalldawson reopened this May 28, 2024
Copy link

Plugin ready!

A test version of this PR is available for testing here.

(Built from commit d4b857c)

@ChrisLoer
Copy link
Contributor

This looks like it's working as intended 🎉

Screenshot 2024-05-28 at 3 16 41 PM

@nyalldawson nyalldawson merged commit c8ae222 into main May 28, 2024
12 checks passed
@nyalldawson nyalldawson deleted the log_fsl branch May 28, 2024 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants