Keda Pulsar integration doc says msgBacklogThreshold is the parameter but code is looking for msgBacklog

[frankjkelly]

Report

Compare https://keda.sh/docs/2.10/scalers/pulsar/
mentioning
msgBacklogThreshold: '5'
to

keda/pkg/scalers/pulsar_scaler.go

Line 43 in 2234a6f

msgBacklogMetricName = "msgBacklog"

Expected Behavior

Code matches documentation

Actual Behavior

Code does not match documentation - overriding msgBacklogThreshold has no impact

Steps to Reproduce the Problem

1. Compare Docs and code

Logs from KEDA operator

N/A

KEDA Version

2.10.1

Kubernetes Version

< 1.23

Platform

Amazon Web Services

Scaler Details

Pulsar

Anything else?

No response

[JorTurFer]

Wow, nice catch!
Other scalers use the suffix "threshold", so I'd update the code... Are you willing to open a PR fixing it? Even if it's not documented, we should maintain both IMO as it's something crucial for the scaler.
I reckon that just updating this code with this could be enough (and adding a tests case):

	// FIXME: msgBacklog support DEPRECATED to be removed in v2.13
	if val, ok := config.TriggerMetadata[msgBacklogMetricName]; ok {
		logger.Info("\"msgBacklog\" is deprecated and will be removed in v2.13, please use \"msgBacklogThreshold\" instead")
		t, err := strconv.ParseInt(val, 10, 64)
		if err != nil {
			return meta, fmt.Errorf("error parsing %s: %w", msgBacklogMetricName, err)
		}
		meta.msgBacklogThreshold = t
	} else if val, ok := config.TriggerMetadata["msgBacklogThreshold"]; ok {
		t, err := strconv.ParseInt(val, 10, 64)
		if err != nil {
			return meta, fmt.Errorf("error parsing %s: %w", msgBacklogMetricName, err)
		}
		meta.msgBacklogThreshold = t
	}
	// END FIXME

I have reduced the code to include a snippet, but the logger should be propagated until here too

[frankjkelly]

@JorTurFer Sorry I'm not great with Golang - if there were already some test cases with msgBacklogThreshold I could probably wing it.

[JorTurFer]

These are unit test for pulsar: https://github.com/kedacore/keda/blob/main/pkg/scalers/pulsar_scaler_test.go
I have quickly checked and there isn't any test checking that (I guess that's been the problem) but it shouldn't be too difficult for being added.
If you don't feel comfortable with golang, don't worry. Let's keep this issue open and wait for community help

[Mutusva]

I am starting out and would like to contribute. I have knowledge of GO is that fine if I open the PR.

[JorTurFer]

Hey!
sure @Mutusva , it'd be nice ❤️

[dttung2905]

Hi @Mutusva, are you still working on this ? I can author a quick PR if you don't mind

[Mutusva]

Hi @Mutusva, are you still working on this ? I can author a quick PR if you don't mind

Hi @dttung2905 sure you can take it up, had travelled and my internet isn't so stable