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

chore(greptimedb_metrics sink): fix greptimedb examples #21984

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

sunng87
Copy link
Contributor

@sunng87 sunng87 commented Dec 9, 2024

Summary

This patch updates examples of greptimedb_metrics

  • corrected example value of gzip_compression
  • add example of new_naming
  • removed redundant example of endpoint

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

N/A

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

  • Please read our Vector contributor resources.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

@sunng87 sunng87 requested review from a team as code owners December 9, 2024 04:52
@github-actions github-actions bot added domain: sinks Anything related to the Vector's sinks domain: external docs Anything related to Vector's external, public documentation labels Dec 9, 2024
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sunng87 !

@jszwedko jszwedko enabled auto-merge December 9, 2024 09:07
@sunng87
Copy link
Contributor Author

sunng87 commented Dec 9, 2024

@jszwedko Thank you for fast response! I have a question. My idea is to change default value of this new_naming to true eventually. But this change will affect existing users, if done wrong, it will break users' data, because this option control how we name timestamp column in GreptimeDB.

My idea is to:

  1. Add a breaking change to greptimedb_metrics sink, to make this option a required one and change default value from false to true. So users will get notified when upgrading from older versions and set correct value of this field.
  2. After several months and a few minor releases of vector, change this option back to optional so new users won't need to know this option.

Do you think this will work? Do you have similar practice at Vector before?

@jszwedko
Copy link
Member

jszwedko commented Dec 9, 2024

@jszwedko Thank you for fast response! I have a question. My idea is to change default value of this new_naming to true eventually. But this change will affect existing users, if done wrong, it will break users' data, because this option control how we name timestamp column in GreptimeDB.

My idea is to:

  1. Add a breaking change to greptimedb_metrics sink, to make this option a required one and change default value from false to true. So users will get notified when upgrading from older versions and set correct value of this field.
  2. After several months and a few minor releases of vector, change this option back to optional so new users won't need to know this option.

Do you think this will work? Do you have similar practice at Vector before?

Hey! Yes, something like that would work. It matches the deprecation process we have documented. See the deprecation process documentation for more details on the process.

@jszwedko jszwedko added the no-changelog Changes in this PR do not need user-facing explanations in the release changelog label Dec 9, 2024
@jszwedko jszwedko changed the title chore(greptimedb): fix greptimedb examples chore(greptimedb sink): fix greptimedb examples Dec 9, 2024
auto-merge was automatically disabled December 10, 2024 00:53

Head branch was pushed to by a user without write access

@jszwedko jszwedko enabled auto-merge December 10, 2024 08:26
@sunng87 sunng87 changed the title chore(greptimedb sink): fix greptimedb examples chore(greptimedb_metrics sink): fix greptimedb examples Dec 10, 2024
@pront
Copy link
Member

pront commented Dec 10, 2024

For https://github.com/vectordotdev/vector/actions/runs/12247198112/job/34178224846?pr=21984:

./website/scripts/cue.sh fmt
CI=true make check-docs

auto-merge was automatically disabled December 11, 2024 02:13

Head branch was pushed to by a user without write access

@sunng87
Copy link
Contributor Author

sunng87 commented Dec 11, 2024

@pront Thank you! It seems example is not allowed for bool fields

@jszwedko jszwedko enabled auto-merge December 11, 2024 09:49
@jszwedko jszwedko added this pull request to the merge queue Dec 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 11, 2024
@pront pront added this pull request to the merge queue Dec 11, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Dec 12, 2024
@jszwedko jszwedko added this pull request to the merge queue Dec 12, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2024
* docs: fix greptimedb example

* ci: add greptimedb sinks to semantic ci

* fix: remove examples for bool field
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@jszwedko jszwedko added this pull request to the merge queue Dec 12, 2024
Merged via the queue into vectordotdev:master with commit 70837f3 Dec 12, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: external docs Anything related to Vector's external, public documentation domain: sinks Anything related to the Vector's sinks no-changelog Changes in this PR do not need user-facing explanations in the release changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants