[fix] Close Elasticsearch client properly

[Lauquik]

Which problem is this PR solving?

• resolves [test] Flaky test TestPasswordFromFile in plugin/storage/es/factory_test.go #4743

Description of the changes

• Resolved an issue where the Elasticsearch client was not being closed correctly.

How was this change tested?

• manually on local env

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: yarn lint and yarn test

[yurishkuro]

why do we need to do manual closing?

[Lauquik]

when i tested it, this was the error that i encountered

--- FAIL: TestPasswordFromFileErrors (0.05s)
    logger.go:130: 2023-09-12T22:33:23.038+0530 INFO    Elasticsearch detected  {"version": 6}
    logger.go:130: 2023-09-12T22:33:23.038+0530 INFO    loaded new password of length 14 from file
    logger.go:130: 2023-09-12T22:33:23.038+0530 ERROR   failed to recreate Elasticsearch client with new password       {"error": "no servers specified"}
    logger.go:130: 2023-09-12T22:33:23.038+0530 INFO    loaded new password of length 14 from file
    logger.go:130: 2023-09-12T22:33:23.038+0530 ERROR   failed to recreate Elasticsearch client with new password       {"error": "no servers specified"}
    logger.go:130: 2023-09-12T22:33:23.038+0530 ERROR   failed to reload password for Elasticsearch client      {"error": "open /tmp/TestPasswordFromFileErrors2482467054/001/pwd: no such file or directory"}
    logger.go:130: 2023-09-12T22:33:23.038+0530 ERROR   failed to reload password for Elasticsearch client      {"error": "open /tmp/TestPasswordFromFileErrors2482467054/001/pwd: no such file or directory"}
    logger.go:130: 2023-09-12T22:33:23.038+0530 INFO    Received event  {"event": "REMOVE        \"/tmp/TestPasswordFromFileErrors2482467054/001/pwd\""}
    logger.go:130: 2023-09-12T22:33:23.038+0530 WARN    Unable to read the file {"file": "/tmp/TestPasswordFromFileErrors2482467054/001/pwd", "error": "open /tmp/TestPasswordFromFileErrors2482467054/001/pwd: no such file or directory"}
    logger.go:130: 2023-09-12T22:33:23.038+0530 ERROR   failed to reload password for Elasticsearch client      {"error": "open /tmp/TestPasswordFromFileErrors2482467054/001/pwd: no such file or directory"}
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xc88bd5]

goroutine 120 [running]:
testing.tRunner.func1.2({0xd89360, 0x1631cc0})
        /usr/local/go/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.

---

i dont' know the exact reason but looks like when clients are closed onClientPasswordChange() function was unable to
loadTokenFromFile and threw the error

[yurishkuro]

is this the full stack trace of the panic? It does not show any of our functions.

[Lauquik]

goroutine 120 [running]:
testing.tRunner.func1.2({0xd89360, 0x1631cc0})
        /usr/local/go/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
        /usr/local/go/src/testing/testing.go:1548 +0x397
panic({0xd89360?, 0x1631cc0?})
        /usr/local/go/src/runtime/panic.go:914 +0x21f
github.com/jaegertracing/jaeger/plugin/storage/es.(*Factory).getArchiveClient(...)
        /mnt/j/cncf/jaeger/plugin/storage/es/factory.go:144
github.com/jaegertracing/jaeger/plugin/storage/es.(*Factory).Close(0xc00023c230)
        /mnt/j/cncf/jaeger/plugin/storage/es/factory.go:292 +0x275
github.com/jaegertracing/jaeger/plugin/storage/es.TestPasswordFromFileErrors(0xc00070e1a0)
        /mnt/j/cncf/jaeger/plugin/storage/es/factory_test.go:386 +0x585
testing.tRunner(0xc00070e1a0, 0xf1c0d8)
        /usr/local/go/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
        /usr/local/go/src/testing/testing.go:1648 +0x3ad
exit status 2
FAIL    github.com/jaegertracing/jaeger/plugin/storage/es       0.207s

---

this is the remaining error log but here onClientPasswordChange is not mentioned i figured it out by myself , i maybe wrong
but after executing defer f.Close() the test calls onClientPasswordChange() this function
and "f.logger.Error("failed to reload password for Elasticsearch client", zap.Error(err)) " this error handle is presenet in that function (onClientPasswordChange)

and we can see the Zap.Error log which says {"error": "no servers specified"}

i tried rerun the test without closing factory resources and the test passed , i manyally closed the watchers and test still passed but after clsoing clients it didn't passed

[yurishkuro]

I was thinking this would need to be more complicated, but I think you're right, as long as we close the watchers above, there's no more concurrency with the clients being swapped, so it's safe to close them like this.
👍

[codecov]

Codecov Report

Patch coverage is 83.33% of modified lines.

Files Changed	Coverage
plugin/storage/es/factory.go	83.33%

📢 Thoughts on this report? Let us know!.

[yurishkuro]

@Lauquik thanks for PR!