-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat: update logcli so it tries to load the latest version of the schemaconfig #11852
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
Conversation
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
Signed-off-by: Michel Hollands <michel.hollands@grafana.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just 2 nits
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
pkg/logcli/query/query.go
Outdated
searchFor := fmt.Sprintf("%s.yaml", schemaConfigFilename) // schemaconfig.yaml for backwards compatibility | ||
loadedSchema, err := LoadSchemaUsingObjectClient(client, searchFor) | ||
if err == nil { | ||
return loadedSchema, nil | ||
} | ||
if err != errNotExists && err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to move this fallback logic to the end of the function.
let's say if we do not have any files with the naming convention schemaconfig-tenant.yaml
or schemaconfig-tenant-N.yaml
, then we fallback and load schemaconfig.yaml
.
with the current implementation, it will always load schemaconfig.yaml
even if newer files exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. This has been moved to the back. Tests have been added as well to check the cases of 1) a single legacy schemaconfig and 2) both a new style one and a legacy one.
Tested it with running logcli against a GCP bucket with bucket retention policy set.
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
Signed-off-by: Michel Hollands <michel.hollands@gmail.com>
…emaconfig (grafana#11852) Signed-off-by: Michel Hollands <michel.hollands@gmail.com> Signed-off-by: Michel Hollands <michel.hollands@grafana.com> Co-authored-by: Dylan Guedes <djmgguedes@gmail.com>
What this PR does / why we need it:
Cloud Logs exporter will write new versions of the schema config in separate files now. This means it will work with object stores that have a policy of write once (ie no updates). Logcli is changed in this PR to take that into account.
Checklist
CONTRIBUTING.md
guide (required)CHANGELOG.md
updatedadd-to-release-notes
labeldocs/sources/setup/upgrade/_index.md
production/helm/loki/Chart.yaml
and updateproduction/helm/loki/CHANGELOG.md
andproduction/helm/loki/README.md
. Example PRdeprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR