feat: Add capability to pre-connect to MQTT Broker for MQTT Export

lenny-goodell commented 11 months ago

Performance enhancement for when service is processing many events very fast causing back ups when using lazy connection.

closes #1516

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/app-functions-sdk-go/blob/main/.github/CONTRIBUTING.md

PR Checklist

Please check if your PR fulfills the following requirements:

[x] I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
[x] I am not introducing a new dependency (add notes below if you are)
[x] I have added unit tests for the new feature or bug fix (if not, why?)
[x] I have fully tested (add details below) this the new feature or bug fix (if not, why?)
[x] I have opened a PR for the related docs change (if not, why?) TBD

Testing Instructions

Run non-secure edgex stack with Device Virtual and MQTT Broker

make run no-secty ds-virtual mqtt-broker

Comment out existing pipeline function init code in app template Add the following pipeline function int code

    export := transforms.NewMQTTSecretSender(mqttConfig, false)
    export.ConnectToBroker(app.service.LoggingClient(), app.service.SecretProvider(), 5, time.Second*3)

    err := app.service.SetDefaultFunctionsPipeline(export.MQTTSend)
    if err != nil {
        app.lc.Errorf("SetFunctionsPipeline returned error: %s", err.Error())
        return -1
    }

Build and run app-template with TRACE logging

 WRITABLE_LOGLEVEL=TRACE ./app-new-service -cp -d

Verify events are being processed and exported

level=DEBUG ts=2023-12-13T23:33:02.999684216Z app=app-new-service source=mqttsecret.go:293 msg="Sent 459 bytes of data to MQTT Broker in pipeline 'defa
ult-pipeline' to topic 'mqtt-export'"

Stop the MQTT Broker verify logs contain

level=TRACE ts=2023-12-13T23:29:13.556428606Z app=app-new-service source=mqttsecret.go:208 msg="MQTT Broker for export lost connection"
level=TRACE ts=2023-12-13T23:29:13.556462306Z app=app-new-service source=mqttsecret.go:213 msg="MQTT Broker for export re-connecting"
level=ERROR ts=2023-12-13T23:32:58.024660412Z app=app-new-service source=triggermessageprocessor.go:154 msg="message error in pipeline default-pipeline
 (c6de0722-9197-4192-99be-9256a64e9918): in pipeline 'default-pipeline', connection to mqtt server for export not open, dropping event"

Restart the MQTT Broker Verify logs contain

level=TRACE ts=2023-12-13T23:32:58.546204672Z app=app-new-service source=mqttsecret.go:204 msg="MQTT Broker for export connected"

Verify events are again being processed and exported

level=DEBUG ts=2023-12-13T23:36:33.088627053Z app=app-new-service source=mqttsecret.go:293 msg="Sent 407 bytes of data to MQTT Broker in pipeline 'default-pipeline' to topic 'mqtt-export'"

New Dependency Instructions (If applicable)

lenny-goodell commented 11 months ago

@cloudxxx8 , this fixes the issue with lazy connect. Had to make it optional and not cause service to exit on start-up to avoid backwards breaking changes. #1526 will address the Store and Forward part.

codecov-commenter commented 11 months ago

Codecov Report

Attention: 57 lines in your changes are missing coverage. Please review.

:exclamation: No coverage uploaded for pull request base (main@f1b4116). Click here to learn what that means.

Files	Patch %	Lines
pkg/transforms/mqttsecret.go	18.00%	41 Missing :warning:
internal/app/configurable.go	63.63%	11 Missing and 5 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1527 +/- ## ======================================= Coverage ? 66.65% ======================================= Files ? 36 Lines ? 3197 Branches ? 0 ======================================= Hits ? 2131 Misses ? 928 Partials ? 138 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cloudxxx8 commented 11 months ago

@judehung please help review this PR

judehung commented 11 months ago

I don't quite sure if this PR can really fixes the OOM as described in #1516. I'll have to make further test--ds-virtual reproducing high frequency data (10 messages per 200 ms) and unreachable MQTT broker--to confirm my suspicion.

UPDATED: Just make a test to have ds-virtual produce high frequency data (10 messages per 200 ms) and unreachable MQTT broker. The MEM USAGE of app-new-service as reported by docker stats looks quite stable without soaring, so my suspicion about OOM can be denied.

lenny-goodell commented 10 months ago

@cloudxxx8 , could you please approve as a committer. THX!

edgexfoundry / app-functions-sdk-go