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.

Sign up for GitHub

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

Jump to bottom

Improve horribly confusing CUE validation failure output #37859

Closed
sdboyer opened this issue Aug 13, 2021 · 4 comments
Closed

Improve horribly confusing CUE validation failure output #37859

sdboyer opened this issue Aug 13, 2021 · 4 comments
Assignees
@sdboyer
Labels
area/thema stale Issue with no recent activity

Comments

@sdboyer
Copy link
Contributor

sdboyer commented Aug 13, 2021 •
edited

NOTE - the error output no longer looks exactly like this, after #38727. Now it's even MORE opaque.

Here's some CUE error output, generated when a devenv dashboard failed to validate:

$ grafana-cli cue validate-resource --dashboard devenv/dev-dashboards/panel-timeline/timeline-demo.json
Error: ✗ Family.lineages.0.0.panels.0: 7 errors in empty disjunction:
Family.lineages.0.0.panels.0: field fieldConfig not allowed:
    /Users/sdboyer/ws/grafana/cue/scuemata/scuemata.cue:26:12
    /cue/data/gen.cue:8:4
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:76:54
    /cue/data/gen.cue:309:28
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:20:7
Family.lineages.0.0.panels.0.fieldConfig.defaults.custom: field fillOpacity not allowed:
    /Users/sdboyer/ws/grafana/cue/scuemata/scuemata.cue:26:12
    /cue/data/gen.cue:8:4
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:294:38
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:26:13
    glue-panelDisjunction:3:2
    glue-panelDisjunction:3:32
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
    text-glue-panelComposition:10:2
    text-glue-panelComposition:15:34
Family.lineages.0.0.panels.0.type: conflicting values "barchart" and "state-timeline":
    /cue/data/gen.cue:76:31
    barchart-glue-panelComposition:3:9
    barchart-glue-panelComposition:11:9
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
Family.lineages.0.0.panels.0.type: conflicting values "graph" and "state-timeline":
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:76:40
    /cue/data/gen.cue:335:27
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15
Family.lineages.0.0.panels.0.type: conflicting values "histogram" and "state-timeline":
    /cue/data/gen.cue:76:31
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
    histogram-glue-panelComposition:3:9
    histogram-glue-panelComposition:11:9
Family.lineages.0.0.panels.0.type: conflicting values "status-history" and "state-timeline":
    /cue/data/gen.cue:76:31
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
    status-history-glue-panelComposition:3:9
    status-history-glue-panelComposition:11:9
Family.lineages.0.0.panels.0.type: conflicting values "timeseries" and "state-timeline":
    /cue/data/gen.cue:76:31
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
    timeseries-glue-panelComposition:3:9
    timeseries-glue-panelComposition:11:9

Horrifying. Not incorrect - but horrifying.

This absolutely must be improved, but that's likely not going to be something we get to right away, so i'm going to break down what's happening here for reference and educational purposes. Spoiler: the actual problem was that no models.cue had yet been defined for the state-timeline plugin, but the artifact contained a state-timeline panel, and (in this, strictest of validation modes) it's not allowed to have a panel of an unknown plugin type.

Main error:

Family.lineages.0.0.panels.0: 7 errors in empty disjunction:

This is the first thing to spot. If you miss it, everything else is a maze. It's telling you that:

  • There's a disjunction (logical or) in the schema having seven branches, and it's in:
  • The concrete object (dashboard JSON) was not valid with respect to any of the seven branches
  • The next seven errors that follow in the output will explain the problem with each of those seven branches

Yeah, this is basically a grouping header, even though its child elements aren't additionally indented. Oh, what a difference whitespace would make.

Suberror/branch 1:

Family.lineages.0.0.panels.0: field fieldConfig not allowed:
    /Users/sdboyer/ws/grafana/cue/scuemata/scuemata.cue:26:12
    /cue/data/gen.cue:8:4
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:76:54
    /cue/data/gen.cue:309:28
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:20:7

What this literally says is that the first branch isn't allowed because the panel artifact contains a field called fieldConfig. But that's uninformative. What's actually important is why it's not allowed, because...uh, what? Of course fieldConfig should be allowed.

But that only becomes obvious if you follow the line numbers. Specifically, [/cue/data/gen.cue:309:28]:(

grafana/cue/data/gen.cue

Line 309 in b89680c

#RowPanel: {
) is the beginning of #RowPanel's definition, and it doesn't allow FieldConfig.

Suberror/branch 2:

Family.lineages.0.0.panels.0.fieldConfig.defaults.custom: field fillOpacity not allowed:
    /Users/sdboyer/ws/grafana/cue/scuemata/scuemata.cue:26:12
    /cue/data/gen.cue:8:4
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:294:38
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:26:13
    glue-panelDisjunction:3:2
    glue-panelDisjunction:3:32
    glue-panelDisjunction:3:34
    glue-unifyPanelDashboard:4:11
    text-glue-panelComposition:10:2
    text-glue-panelComposition:15:34

This one seems promising - it appears there's an extra field (fillOpacity) on line 26 of the artifact ([devenv/dev-dashboards/panel-timeline/timeline-demo.json:26](https://github.com/grafana/grafana/blob/aef67994a18cf8f036ae5e0310d066dea1857132/devenv/dev-dashboards/panel-timeline/timeline-demo.json#L26)) that the plugin doesn't specify in its schema!

But it's not. It's lies. LIES. Honestly, i'm not sure why this one comes out the way it does - it should look like errors 3, 5, 6, and 7. But here's a big clue: text-glue-panelComposition:15:34. These line numbers are a product of how CUE internally tracks the exact source of every value it knows about, including through all mappings and references and such. It's one of CUE's most important properties, especially at scale.

But that's not a real file - rather, it's an intermediate, dynamically generated helper we have in our Go code that glues the panel schema onto the base dashboard schema. What's important is the name: text is the name of the plugin being glued on. If you look over the other errors, you'll see they each have a different name in that spot.

So, while the error here is true - the text plugin doesn't allow fillOpacity - what'd be useful is talking about the type field. Which is what most of the others do.

Suberrors/branches 3, 5, 6, 7:

Family.lineages.0.0.panels.0.type: conflicting values "barchart" and "state-timeline":
(...etc.)

This is what would've been more useful to see - the type value on the schema branch is barchart, but that conflicts with what's in the artifact, because the panel being validated is a state-timeline, not a barchart. Same deal with the other three.

Suberror/branch 4:

Family.lineages.0.0.panels.0.type: conflicting values "graph" and "state-timeline":
    /cue/data/gen.cue:76:31
    /cue/data/gen.cue:76:40
    /cue/data/gen.cue:335:27
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:1:1
    devenv/dev-dashboards/panel-timeline/timeline-demo.json:110:15

This looks mostly the same as the others, except that there's no line numbers for the compositional glue. That's because #GraphPanel is hardcoded into the dashboard schema. We do that because the old graph panel wasn't well-behaved about keeping its config within the options and fieldConfig spaces, which breaks our standard composition mechanism. Fortunately, because the graph plugin is deprecated and not going to receive updates, we can just write it directly into the dashboard scuemata as a one-off and not worry about it again.

What to do to improve it?

I'm not sure just right offhand. We might be able to fix up the schema a bit to make the problem with branch 2 better. We could also take a more drastic step of running separate validation processes from Go, though complexity and maintenance cost there would be fairly high. Best bet is probably spelunking the errors when they come through and sniffing for some known patterns in order to remove a bunch of noise from the output. This one, at least, ought to be reducible to simply "no known panel plugin of type state-timeline".
@sdboyer sdboyer self-assigned this Aug 13, 2021
@sdboyer
Copy link
Contributor Author

sdboyer commented Aug 30, 2021 •
edited

related core issue: cue-lang/cue#602; seems likely that efforts on our end without a lang-level resolution will be fairly brittle and high-investment

@sdboyer
Copy link
Contributor Author

sdboyer commented Sep 14, 2021

Unfortunately, #38727 has made this problem even worse - error output no longer seems to include output that traces back to the original models.cue file of a plugin 😠

@sdboyer sdboyer mentioned this issue Sep 16, 2021
Closed
13 tasks
@sdboyer sdboyer mentioned this issue Sep 28, 2021
Merged
@sdboyer sdboyer mentioned this issue Nov 22, 2021
Closed
@zuchka zuchka mentioned this issue Dec 27, 2021
Closed
@sdboyer sdboyer mentioned this issue Apr 24, 2022
Closed
3 tasks
@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had activity in the last year. It will be closed in 30 days if no further activity occurs. Please feel free to leave a comment if you believe the issue is still relevant. Thank you for your contributions!

@github-actions github-actions bot added the stale Issue with no recent activity label Jan 28, 2024
@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically closed because it has not had any further activity in the last 30 days. Thank you for your contributions!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sdboyer sdboyer

Labels
area/thema stale Issue with no recent activity
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@sdboyer