aws-observability / amazon-managed-grafana-migrator

CLI migration utility to migrate Grafana content to Amazon Managed Grafana
Apache License 2.0
25 stars 7 forks source link

[Bug]: grafana go sdk cannot unmarshal number -1 into Go struct field FolderDashboardSearchResponse.folderId of type uint #21

Closed sjoukedv closed 4 months ago

sjoukedv commented 10 months ago

Did you search for similar issues before submitting?

Release version

0.1.9

Is it a permission issue?

What is your environment, configuration and the example used?

Two grafana 8.4 instances trying to migrate from src to dst in order to upgrade the 'secondary' workspace first and be able to validate 9.4 works for us.

Additional Information

migrate throws an error for migrating dashboards. Seems to be thrown by https://github.com/aws-observability/amazon-managed-grafana-migrator/blob/main/internal/pkg/app/dashboards.go#L13

✔ Migrated 6 folders

Migrating dashboards:

Removing temporary API key for g-<src>

Removing temporary API key for g-<dst>
error: json: cannot unmarshal number -1 into Go struct field FolderDashboardSearchResponse.folderId of type uint
bonclay7 commented 10 months ago

Hey there, seems like one of your folders got somehow a negative id, Did the migration completed all the folders at least?

sjoukedv commented 10 months ago

Hey there, seems like one of your folders got somehow a negative id, Did the migration completed all the folders at least?

No, it did not. Some things seemed to be corrupted with the Grafana instance. After a fresh install this worked perfectly fine again. Can we catch the error, log it, and continue the rest of the migration?

bonclay7 commented 10 months ago

@sjoukedv Fixing this is a bit more delicate than I thought. I'm unsure of the state of the objects after one of those errors is thrown.

From the client used here https://github.com/grafana/grafana-api-golang-client/blob/master/client.go#L149-L152

seems like nothing is returned if the parsing fails. So the fix has to be upstream (maybe just a type change here

1/ Do you have a way to reproduce that error? 2/ would it make sense to open this issue on the sdk repo?

What we can do so far is catch that error, still exiting but with a clearer message for now as dashboards are the last step of the migration (until alerting is solved)