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

Adding cosmos encryption benchmarking #23268

Merged

Conversation

simplynaveen20
Copy link
Member

Created separate package for testing encryption scenarios.

Issue found during this activity - Aggregation query is not supported #23160

Below are some metrics

image
image
image

Copy link
Contributor

@moderakh moderakh left a comment

Choose a reason for hiding this comment

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

LGTM. awesome work @simplynaveen20

else {
asyncBenchmark(cfg);
} else {
if (cfg.isEncryptionEnabled()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we follow the same pattern as the other else if here?

else if (cfg.isEncryptionEnabled()) 

instead of

else {
   if (cfg.isEncryptionEnabled()) 
}

Copy link
Member Author

Choose a reason for hiding this comment

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

done

#TenantId = REPLACEME
#ClientId = REPLACEME
#ClientSecret = REPLACEME
#KeyVaultMasterKeyUrl = https://REPLACEME.vault.azure.net/keys/REPLACEME/REPLACEME
Copy link
Contributor

Choose a reason for hiding this comment

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

benchmark tool at the end runs as a standalone jar, how does this template file in the resource folder helps?

shouldn't this be outside of the source code, maybe in a small docs folder?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the method which is consuming this to non static , now this file will be accessed directly by the code via AsyncEncryptionBenchmark.class.getClassLoader().getResourceAsStream("encryption_setting.properties");) without having this file manually need to put on the machine where the jar is

Copy link
Contributor

@moderakh moderakh Jul 30, 2021

Choose a reason for hiding this comment

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

if the jar is already created, then this file in the template format is in the jar without the proper uri, etc (you have replaceme toekn) which means the end user has to modify the file in the jar.

the configuration has to be independent of the binary. that's the reason I suggested you should have this file outside of the source code.

@simplynaveen20 any specific reason you want this file to be in the source code and not in some docs or config folder outside of the source code?

Copy link
Member Author

@simplynaveen20 simplynaveen20 Jul 30, 2021

Choose a reason for hiding this comment

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

This is the code resource so it should go under resource. User can change the file and create the single jar ,then he need not to worry about putting it explicitly in the same local location as jar, this will avoid overhead.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, thanks Naveen for the explanation.

@simplynaveen20 simplynaveen20 merged commit a9c7cd4 into Azure:main Aug 3, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-java that referenced this pull request Apr 18, 2023
Feature/cplat 2023 03 01 (Azure#22885)

* copy last version to 2023-03-01 folder

* update version reference in the new folder

* update versions reference in the new folder

* add new readme tag with new version

* sync with updates made to 2022-11-01 version

* fix generation error

* fix duplicate enum name thru c# directive instead.

* make all the changes to this version that was made to the last version (2022-11-01) to address modelvalidation errors

* update

* Revert "update"

This reverts commit 08417d3ed412ab63c7ee7bdb712ad756ded03c97.

* Added $expand option in ListAllVMs and ListAllVMs in RG (Azure#22800)

* Added $expand option in ListAllVMs and ListAllVMs in RG

* Update virtualMachine.json

* dedicatedHost Resize feature (Azure#23268)

* dedicatedHost Resize feature

* DedicatedHost Sku renamed to size and resolved lint errors.

* reviewer comments

* httpMethod fix

* Add property for VM (Azure#22882) (Azure#23329)

Co-authored-by: payalguptapg <126145083+payalguptapg@users.noreply.github.com>

* Add Reapply for VMSS (Azure#22344)

* Add Reapply for VMSS

* Prettier fix

* Update examples

* Address review comments 01

* Use typical resourceGroupName parameter.

* Address review comments - Rename examples

* Remove <br> syntax from many descriptions in CRP swagger files (Azure#23019)

* up to computeRPCommon

* all but computeRPCommons

* computerpcommon

* vmss clean

* common clean

* vmss try n

* trying only \n

* remove \n as it messes with rest docs

* cleanup 2022-11-01 accidents

* cleanups 2023

* remove ID from Update objects that do not have ID (Azure#23078)

* update

* add identifiers

---------

Co-authored-by: Theodore Chang <theodore.l.chang@gmail.com>

* Origin/feature/cplat 2023 03 01 (Azure#23203)

* Update virtualMachine.json

Added missing property for 
Additional properties not allowed: provisionAfterExtensions
Json path: $.value[*].properties.provisionAfterExtensions

* added missing property for provisionAfterExtensions

* add locatoin in VirtualMachineScaleSetVMExtension

* fixed issue of x-ms-mutability

---------

Co-authored-by: Younghyun Kim <younghyunkim@microsoft.com>

* Add securityPostureReference (Azure#23106)

* [RestorePoints] Adding Encryption, Source Details, HyperVGeneration and WriteAccelerated for Restore Point (Azure#23303)

* Restore Point Encryption Details

* Modified descriptions and made DiskRestorePointProperties as input property

* Renaming object and adding type

* Renaming DiskRestorePointProperties

* making DiskRestorePoint.id readOnly

* Modifying reference

* Running Prettier

* Renaming EncryptionType

* Adding HyperVGeneration and WriteAcceleratorEnabled (prchin)

* Add optional parameter hibernate to vmss deallocate api (Azure#23409)

* Add optional parameter hibernate to vmss deallocate api

* Fix with prettier

* update example for vmss deallocate

* Add Spot Related Properties to VMSS PATCH

* prettier and lintDiff suppressions

* retry lintDiff suppression

* lint diff suppression retry

* Add managed identities parameters for blobs, add treatFailureAsDeploymentFailure flag for Run Command (Azure#23557)

* Add managed identities inputs for script, errorBlob, outputBlob

* Add treatFailureAsDeploymentFailure flag

* Update proximityPlacementGroup.json (Azure#23556)

* Update proximityPlacementGroup.json

* Update proximityPlacementGroup.json

* Update proximityPlacementGroup.json

---------

Co-authored-by: Theodore Chang <theodore.l.chang@gmail.com>
Co-authored-by: Kartik Gupta <31189137+Kartik-715@users.noreply.github.com>
Co-authored-by: cakarata <47228825+cakarata@users.noreply.github.com>
Co-authored-by: payalguptapg <126145083+payalguptapg@users.noreply.github.com>
Co-authored-by: Anshul Verma <88476874+AnshulVermaa@users.noreply.github.com>
Co-authored-by: Adam Sandor <adsandor@microsoft.com>
Co-authored-by: younghyun5756 <102988755+younghyun5756@users.noreply.github.com>
Co-authored-by: Younghyun Kim <younghyunkim@microsoft.com>
Co-authored-by: krishnak-msft <127885427+krishnak-msft@users.noreply.github.com>
Co-authored-by: Linu George <127192938+linugeorgeofficial@users.noreply.github.com>
Co-authored-by: vatsan28 <vatsan9228@gmail.com>
Co-authored-by: Viv Lingaiah <viv.lingaiah@gmail.com>
Co-authored-by: Micah McKittrick <32313503+mimckitt@users.noreply.github.com>
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.

3 participants