language agnostic checkpointing for azure eventhub scaler

[christle]

this is a solution for a language agnostic checkpointing.

It comes with a new konfiguration for eventhub scaler: checkpointType.

Actually with 3 Types:

AzureWebJob: the is default if no blobcontainer is set. this is the current working behavior for azure functions

GoSdk: GoSdk has a very special storage path and a special checkpoint structure.

Default: Default is set if a blobcontainer is set. Default, because this path and checkpoint structure is used by java, python and maybe others. It stores the values as metadata on the checkpoint blob.

Fixes #769 #767 #741

[ahmelsayed]

Thanks @christle for the contribution. My main concern is that this is a breaking change for eventhub scaler. @tomkerkhove would probably know better than me how we should handle such breaking changes.

[christle]

As I understand, the azure functions are currently the only checkpointing that works. This is the type AzureWebJob and for that, everything stays the same. No additional configuration is necessary either. All other frameworks are currently not executable. But it may be that I'm wrong. If there are currently still variants, they use probably this old Python checkpointing behavior. If want to support old Phyton checkpointing we could add a specific python checkpointer and set it as the default behavior if a container name is set.

[christle]

i think the point with the old python behavior is not bad. I could add it as an additional backward checkpointer. Then we make sure all stays the same. But if anybody will use checkpointing with die current python- dapr- java- eventhub framework and so on, then the checkpointType have to be set explcitly, in this case.

[tomkerkhove]

As I understand, the azure functions are currently the only checkpointing that works. This is the type AzureWebJob and for that, everything stays the same. No additional configuration is necessary either. All other frameworks are currently not executable. But it may be that I'm wrong. If there are currently still variants, they use probably this old Python checkpointing behavior. If want to support old Phyton checkpointing we could add a specific python checkpointer and set it as the default behavior if a container name is set.

That is not correct, actually.

As per the docs:

As of now, the Event Hub scaler only supports reading checkpoints from Blob Storage, as well as scaling only Event Hub applications written in C#, Java, Python or created with Azure Functions.

We'll need to be backwards compatible with all of them.

[christle]

The current implementation in 2.1 is this:

containerName == "" -> use Webjob Behavior (azure function) -> that means use "azure-webjobs-event" as containername
containerName != "" -> use the same checkpointing like above but with a different path (that was suitable for c#, java before the change inside the checkpoint behavior see #767)
when read checkpoint from blob -> try it for default one (both above) and try it for specific python.

currently my change is:
containername == "" or checkpointType is "AzureWebJob" -> use Webjob Behavior (azure function) > that means use "azure-webjobs-event" as containername and the checkpointing is the same like before
containername != "" or checkpointType is "Default" -> use the new path for java and python eventhub sdks (and maybe others, i have to check c# )and read the checkpoint form blob metadata
containername != "" AND checkpointType is "GoSdk" use special gosdk path and checkpointing (also for dapr apps)

what i can add to make it backwards compatible even for old sdks:
containername != "" AND checkpointType is "" -> use an additional checkpointer for older java sdks and the python part, with the try to merge statement on the checkpoint struct. (But then the checkpointing Java, Python with the sdk have to explicit set the checkpointType. That should not be problem because this currently not working in 2.1 so they have to update keda)

i think then we have all former SDK's supported and additional the new checkpointing for java, python + the gosdk. And a open structure to support new checkpointer for more languages which special checkpoint behavior. But i think there will be more sdks that use it like the new Java and Python checkpointer. So i think the suggested default checkpointer support more than these both at the moment.

[christle]

hi @ahmelsayed,

i've added a new checkpointer for backward compatibility. Now there a 4 checkpointer:

DefaultCheckpointer (Old Java SDK, Old Python SDK, C#, maybe more)
this checkpointer handle all checkpoint formats and storage paths, from the current release. if no CheckpointType is configured, this Checkpointer is active.

AzureWebJobCheckpointer (Azure Functions)
this checkpointer handle the format and storage path for azure functions. Works in the same way like before, if no containername is configured (or the checkpointTypeis set to AzureWebJob but this is not required because the backward compatibility).

BlobMetadataCheckpointer (Current Java SDK, Current Python, maybe more)
new checkpointer which reads checkpoint data from blob metadata, and not from the body. I've tested java and python. Maybe there are more eventhub sdk's that use metadata instead of the body. For this behavior the checkpointType: BlobMetadata and containerName are required. fixes #767 #741

GoSdkCheckpointer (GoSdk and Dapr for example which use the GoSdk)
new checkpoint for special golang path and format. For this behavior the checkpointType: GoSdk and containerName are required.

new Checkpointer could be added at the same way. Only the implementation of this interface is required:

type checkpointer interface {
	resolvePath(info azure.EventHubInfo) (*url.URL, error)
	extractCheckpoint(get *azblob.DownloadResponse) (Checkpoint, error)
}

could review that? i would left the PR on DRAFT, because i've already tested all checkpointer on a real storage blob and integrationtests but i want to test it with a complete keda build inside our cluster. Then i can remove the draft, from my side.

[tomkerkhove]

These are a lot of different types to checkpoint, which is fine for me!

However, instead of being so implicit, I would introduce checkpointingStrategy: {strategyName} to be more explicit as this is pretty important.

Wdyt @ahmelsayed @zroubalik?

[tomkerkhove]

The logic itself and all looks great btw, thanks!

[christle]

i just renamed checkpointType to checkpointStrategy. The values are unchanged. Is this ok?

[tomkerkhove]

Fine for me, what do you think @zroubalik @ahmelsayed?

[zroubalik]

Looking good!

[christle]

i just removed the draft flag. Yesterday i've made a successful live test on our dev cluster.

[tomkerkhove]

Would you mind:

• Updating the changelog
• Opening a PR for our docs (https://github.com/kedacore/keda-docs)
• Fix the static checks please?

[christle]

done. I move over to the docs repo

[christle]

the docs PR: kedacore/keda-docs#429

[nzthiago]

For JavaScript the Azure SDK for JS provides a BlobCheckpointStore class which implements the required read/writes to a durable store by using Azure Blob Storage in case you want to see if this implementation would also work for JS.

[christle]

i changed the code to lowercase for checkpointStrategy parameters, @zroubalik

@nzthiago thanks, for your link! I've checked the implementation and it looks like the JS framework works with the "BlobMetadata" checkpointStrategy like Java and Python.

[zroubalik]

Thanks!

[zroubalik]

@tomkerkhove ok to merge this?

[christle]

fixed merge conflict on CHANGELOG.md