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

enhancement: azure_blob sink #6861

Merged
merged 43 commits into from Jun 25, 2021
Merged

enhancement: azure_blob sink #6861

merged 43 commits into from Jun 25, 2021

Conversation

ArtemTrofimushkin
Copy link
Contributor

enhancement(azure_blob_sink): Draft implementation for azure_blob sink

@ArtemTrofimushkin ArtemTrofimushkin mentioned this pull request Mar 23, 2021
Closed
@ArtemTrofimushkin ArtemTrofimushkin changed the title Feature/azure blob sink enhancement: azure_blob sink Mar 23, 2021
@jszwedko
Copy link
Member

Thanks for contributing this @ArtemTrofimushkin ! We can review whenever it is ready. Let us know if you have any questions in the meanwhile.

@ArtemTrofimushkin
Copy link
Contributor Author

ArtemTrofimushkin commented Mar 27, 2021 •
edited

Hi, @jszwedko!
Yes, I have several questions about implementation.

  1. There are two versions of Azure SDK for Rust:
  • Fist one here. This package exists in crates.io, but it's deprecated according to the readme.
  • Second one here. This is a logical successor of the first library, but it does not exist in crates.io, hasn't any releases, and under heavy development right now.
    That's why I chose the first option. Is it ok?
    After the stable release of the second library, I would glad to make another contribution :)
  1. I've implemented several integration tests using Azure Blob Storage Emulator Azurite.
    But two of them always fail, cause of this issue.
    in short, it can be described as follows:
    • Storage emulator contains a bug in the calculation of SAS signature (described in issue).
    • When specified Content-Encoding or Content-Language headers (when Gzip compression enabled), signature calculated incorrectly
    • Blob service unable to authenticate the request
    • Integration test fails

I also mentioned authors of the library about this behavior. What can I do with these tests? Should I leave them, disable them, document this behavior, or completely remove them?

@ArtemTrofimushkin ArtemTrofimushkin marked this pull request as ready for review April 5, 2021 14:45
@ArtemTrofimushkin ArtemTrofimushkin requested review from a team, eeyun, jszwedko and prognant and removed request for a team April 5, 2021 14:45
@binarylogic binarylogic removed the request for review from eeyun April 20, 2021 14:13
@ArtemTrofimushkin
add empty sink & required imports
52e23be 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
implement draft version for azure_blob sink
5eaf45b 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): Determine content_type from compression
0819290 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): Implement custom health check errors
5678646 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): Implement instrumentation
8c3b0c9 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): Implement retries
5cbbd3a 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): Format code & implement text encoding
7a7a37f 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): refactor config options
d8b9949 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): implement append_uuid to blob_name
2f734ad 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): implement some unit tests
67965c7 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): setup integration tests
785d1e7 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): implement integration test for blob wit…
86f941e 
…hout compression

Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): add azure-integration-test make target
c2ab937 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): add azure-blob target to tests & fix fo…
a3a7a5a 
…rmatting

Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
enhancement(azure_blob_sink): fix content encoding handling
5c95c94 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
Copy link
Contributor Author

Thanks, @jszwedko!
Sure, I try to write docs for this sink soon

@ArtemTrofimushkin
enhancement(azure_blob_sink): add draft for docs reference & update d…
eea8a28 
…efault value for blob_prefix

Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
Copy link
Contributor Author

@jszwedko I've pushed changes for documentation. Can you review it, please?

@ArtemTrofimushkin ArtemTrofimushkin requested review from jszwedko and removed request for a team June 24, 2021 21:10
jszwedko
jszwedko reviewed Jun 24, 2021
View reviewed changes
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.

Nice, thanks @ArtemTrofimushkin ! I left a few last comments, but this looks good to me. Thanks for adding the docs!

src/sinks/azure_blob.rs Outdated Show resolved Hide resolved
docs/reference/components/sinks/azure_blob.cue Outdated Show resolved Hide resolved
docs/reference/components/sinks/azure_blob.cue Outdated Show resolved Hide resolved
@ArtemTrofimushkin
enhancement(azure_blob_sink): fix docs & pr comments
5fb5d8f 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
@ArtemTrofimushkin
Copy link
Contributor Author

@jszwedko The corresponding changes pushed :)

jszwedko
jszwedko approved these changes Jun 24, 2021
View reviewed changes
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.

Looks good, thanks for this contribution @ArtemTrofimushkin !

@jszwedko jszwedko enabled auto-merge (squash) June 24, 2021 23:13
auto-merge was automatically disabled June 24, 2021 23:47

Head branch was pushed to by a user without write access

@ArtemTrofimushkin
enhancement(azure_blob_sink): fix docs
4e56d10 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
jszwedko
jszwedko approved these changes Jun 25, 2021
View reviewed changes
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.

Looks good, thanks @ArtemTrofimushkin !

@jszwedko jszwedko enabled auto-merge (squash) June 25, 2021 13:58
@ArtemTrofimushkin
enhancement(azure_blob_sink): fix docs
f2ea1d9 
Signed-off-by: ArtemTrofimushkin <artemtrofimushkin@gmail.com>
auto-merge was automatically disabled June 25, 2021 14:15

Head branch was pushed to by a user without write access

@ArtemTrofimushkin
Copy link
Contributor Author

@jszwedko Thanks!
But it fails on check-docs (formatting issue, my fault, sorry).
Fixed this one.

@jszwedko jszwedko enabled auto-merge (squash) June 25, 2021 15:05
@ArtemTrofimushkin
Copy link
Contributor Author

@jszwedko Is anything else required?
It seems that all checks passed, except Cross - ${{ matrix.target }}.
Github reports, that this check doesn't start yet, but from the last success action all cross builds competed successfully.

@jszwedko
Copy link
Member

Yeah, that's weird, I'm not sure why the status didn't update here. I'll merge manually. Thanks again for this contribution @ArtemTrofimushkin !

@jszwedko jszwedko disabled auto-merge June 25, 2021 21:12
@jszwedko jszwedko merged commit 273e3e1 into vectordotdev:master Jun 25, 2021
@ArtemTrofimushkin
Copy link
Contributor Author

Thanks for help!

@jszwedko jszwedko mentioned this pull request Jun 25, 2021
Merged
@ArtemTrofimushkin ArtemTrofimushkin deleted the feature/azure-blob-sink branch June 25, 2021 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruceg bruceg bruceg approved these changes

@jszwedko jszwedko jszwedko approved these changes

@prognant prognant Awaiting requested review from prognant

@pablosichert pablosichert Awaiting requested review from pablosichert

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ArtemTrofimushkin @jszwedko @bruceg