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

storage: make Azure blobID chunk delimiter configurable #5777

Merged
merged 5 commits into from
Apr 7, 2022

Conversation

tatchiuleung
Copy link
Contributor

@tatchiuleung tatchiuleung commented Apr 5, 2022

What this PR does / why we need it:
Make the - chunk delimiter configurable, instead of hardcoded one -.

Which issue(s) this PR fixes:
Fixes #5712

Special notes for your reviewer:
@dannykopping @khaines I would appreciate if you can review it. Thank you!

Checklist

  • Documentation added
  • Tests updated
  • Add an entry in the CHANGELOG.md about the changes.

@CLAassistant
Copy link

CLAassistant commented Apr 5, 2022

CLA assistant check
All committers have signed the CLA.

@tatchiuleung
Copy link
Contributor Author

@ronyrv13 Feel free to add or change anything in this PR.

@tatchiuleung tatchiuleung force-pushed the fix/azure-chunk-delimiter branch from 72f69af to 800519a Compare April 5, 2022 20:35
@ronyrv13
Copy link

ronyrv13 commented Apr 6, 2022

Hello @tatchiuleung

I have tested loki with this PR changes, and it really worked.
I am able to pull the old logs & new logs with chunk_delimiter config set to ":" with azure storage config.

You can find few SS below-
image

Old Logs Chunks which was successfully fetched -

image

chunk_delimiter config set to "-"

image

Latest logs also Fetched successfully-

image

cc: @dannykopping

@ronyrv13
Copy link

ronyrv13 commented Apr 6, 2022

Loki Config Used-

    storage_config:
        azure:
          container_name: ${CONTAINER_NAME}
          account_name: ${STORAGE_ACCOUNT_NAME}
          account_key: ${STORAGE_ACCOUNT_KEY}
          chunk_delimiter: ":"
        boltdb_shipper:
            shared_store: azure
        index_queries_cache_config:

@tatchiuleung tatchiuleung marked this pull request as ready for review April 6, 2022 14:10
@tatchiuleung tatchiuleung requested review from KMiller-Grafana and a team as code owners April 6, 2022 14:10
@tatchiuleung
Copy link
Contributor Author

@ronyrv13 Thank you for the test!

tatchiuleung added a commit to tatchiuleung/loki that referenced this pull request Apr 6, 2022
Copy link
Contributor

@sandeepsukhani sandeepsukhani left a comment

Choose a reason for hiding this comment

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

LGTM

@sandeepsukhani
Copy link
Contributor

Thanks for the PR! Can you please resolve conflicts with changelog so that I can merge this?

@tatchiuleung
Copy link
Contributor Author

@sandeepsukhani Done!

@sandeepsukhani sandeepsukhani merged commit e65f26d into grafana:main Apr 7, 2022
@dannykopping
Copy link
Contributor

Apologies for not reviewing this PR - I was on vacation this week.
This PR is exactly why I love open-source... You found an issue, reported it, posted a PR to fix, and made the product better ❤️

Thanks a lot!

@prasadrajesh
Copy link

Thanks @tatchiuleung and @ronyrv13 for contribution to make working chunk_delimiter: ":".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure blob storage client should not change key format
6 participants