HPCC-32717 Create basic Elastic sink component #19158

kenrowland · 2024-09-26T16:54:16Z

Created new skeleton component and added to the build

Signed-Off-By: Kenneth Rowland [email protected]

Type of change:

This change is a bug fix (non-breaking change which fixes an issue).
This change is a new feature (non-breaking change which adds functionality).
This change improves the code (refactor or other change that does not change the functionality)
This change fixes warnings (the fix does not alter the functionality or the generated code)
This change is a breaking change (fix or feature that will cause existing behavior to change).
This change alters the query API (existing queries will have to be recompiled)

Checklist:

Smoketest:

Send notifications about my Pull Request position in Smoketest queue.
Test my draft Pull Request.

Testing:

github-actions · 2024-09-26T16:54:33Z

Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-32717

Jirabot Action Result:
Assigning user: [email protected]
Workflow Transition To: Merge Pending
Updated PR

rpastrana

@kenrowland seems reasonable shell. Spotted a few minor issues.

rpastrana · 2024-09-26T17:22:42Z

system/metrics/sinks/elastic/elasticSink.hpp

@@ -0,0 +1,59 @@
+/*##############################################################################
+    HPCC SYSTEMS software Copyright (C) 2021 HPCC Systems®.


wrong date.

rpastrana · 2024-09-26T17:25:14Z

system/metrics/sinks/elastic/elasticSink.cpp

@@ -0,0 +1,53 @@
+/*##############################################################################
+    HPCC SYSTEMS software Copyright (C) 2021 HPCC Systems®.


rpastrana · 2024-09-26T17:25:31Z

system/metrics/sinks/elastic/elasticSink.cpp

+#include <cstdio>
+#include "platform.h"
+
+


remove extra new lines

rpastrana · 2024-09-26T17:26:46Z

system/metrics/sinks/elastic/elasticSink.hpp

+//  doesn't work with format-nonliteral as an error
+//
+#if defined(__clang__) || defined(__GNUC__)
+#pragma GCC diagnostic push


are all of these necessary?

If not included, the following build errors are generated:

In file included from /home/krowland/hpcc/src/system/metrics/sinks/elastic/elasticSink.hpp:30,
from /home/krowland/hpcc/src/system/metrics/sinks/elastic/elasticSink.cpp:14:
/home/krowland/hpcc/src/system/httplib/httplib.h: In instantiation of ‘ssize_t httplib::Stream::write_format(const char*, const Args& ...) [with Args = {int, const char*}; ssize_t = long int]’:
/home/krowland/hpcc/src/system/httplib/httplib.h:3940:61: required from here
/home/krowland/hpcc/src/system/httplib/httplib.h:3635:21: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
3635 | auto sn = snprintf(buf.data(), buf.size() - 1, fmt, args...);
| ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/krowland/hpcc/src/system/httplib/httplib.h:3652:19: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
3652 | snprintf(&glowable_buf[0], glowable_buf.size() - 1, fmt, args...));
| ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/krowland/hpcc/src/system/httplib/httplib.h: In instantiation of ‘ssize_t httplib::Stream::write_format(const char*, const Args& ...) [with Args = {const char*, const char*}; ssize_t = long int]’:
/home/krowland/hpcc/src/system/httplib/httplib.h:4805:76: required from here
/home/krowland/hpcc/src/system/httplib/httplib.h:3635:21: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
3635 | auto sn = snprintf(buf.data(), buf.size() - 1, fmt, args...);
| ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/krowland/hpcc/src/system/httplib/httplib.h:3652:19: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
3652 | snprintf(&glowable_buf[0], glowable_buf.size() - 1, fmt, args...));
| ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There is a similar error w/o the #undef for INVALID_SOCKET

kenrowland · 2024-09-26T20:20:36Z

@ghalliday please merge

ghalliday

@kenrowland a few minor issues to clean up.

ghalliday · 2024-09-30T13:01:43Z

system/metrics/sinks/CMakeLists.txt

@@ -1,5 +1,5 @@
 ###############################################################################
-#    HPCC SYSTEMS software Copyright (C) 2021 HPCC Systems®.
+#    HPCC SYSTEMS software Copyright (C) 20214HPCC Systems®.


trivial: typo in the edit

ghalliday · 2024-09-30T13:02:18Z

system/metrics/sinks/elastic/elasticSink.cpp

+
+
+ElasticMetricSink::ElasticMetricSink(const char *name, const IPropertyTree *pSettingsTree) :
+    PeriodicMetricSink(name, "file", pSettingsTree),


"file" -> "elastic"

ghalliday · 2024-09-30T13:03:27Z

system/metrics/sinks/elastic/elasticSink.cpp

+
+ElasticMetricSink::ElasticMetricSink(const char *name, const IPropertyTree *pSettingsTree) :
+    PeriodicMetricSink(name, "file", pSettingsTree),
+    ignoreZeroMetrics(false)


style: less error prone to use a default initializer on the member definition instead.

ghalliday · 2024-09-30T13:05:47Z

system/metrics/sinks/elastic/elasticSink.hpp

+#endif
+
+#undef INVALID_SOCKET
+#include "httplib.h"


Why are these #includes needed in the header file?

The implementation should be private, and if the class implementation requires members, publish a factory method instead.

I moved #include "nlohmann/json.hpp" to the cpp file since it may not be used for a class member variable.

However, I left httplib since I currently plan on adding a protected member variable for a class defined in that header.

As I move forward on the next step, it looks like a protected member variable will not be the direction. The include was moved to the spp file.

ghalliday · 2024-09-30T13:07:07Z

system/metrics/sinks/elastic/elasticSink.hpp

+    ~ElasticMetricSink() override = default;
+
+protected:
+    void prepareToStartCollecting() override;


style: The rest of the code base includes the virtual qualifier on override functions, even though it is not technically required.

ghalliday · 2024-10-03T16:40:17Z

system/metrics/sinks/elastic/elasticSink.cpp.bak

@@ -0,0 +1,80 @@
+/*##############################################################################


This file was added by mistake - please remove.

Whoops, removed.

ghalliday · 2024-10-09T09:15:09Z

system/metrics/sinks/elastic/elasticSink.hpp

+#include "jptree.hpp"
+#include "jstring.hpp"
+
+#ifdef ELASTICINK_EXPORTS


typo: should be ELASTICSINK_EXPORTS

This is causing the windows build error

Created new skeleton component and added to the build Signed-Off-By: Kenneth Rowland [email protected]

kenrowland requested a review from rpastrana September 26, 2024 17:13

rpastrana reviewed Sep 26, 2024

View reviewed changes

kenrowland requested a review from rpastrana September 26, 2024 18:24

rpastrana approved these changes Sep 26, 2024

View reviewed changes

kenrowland force-pushed the HPCC-32717 branch from 03c70e8 to 3b20b29 Compare September 26, 2024 20:20

ghalliday requested changes Sep 30, 2024

View reviewed changes

kenrowland requested a review from ghalliday September 30, 2024 17:47

kenrowland force-pushed the HPCC-32717 branch from 4e7b1f5 to 8f1fd35 Compare October 1, 2024 19:21

ghalliday reviewed Oct 4, 2024

View reviewed changes

kenrowland force-pushed the HPCC-32717 branch 2 times, most recently from 2f4bd62 to 0112aa8 Compare October 4, 2024 12:44

kenrowland requested a review from ghalliday October 4, 2024 12:46

ghalliday requested changes Oct 9, 2024

View reviewed changes

HPCC-32717 Create basic Elastic sink component

2535bc8

Created new skeleton component and added to the build Signed-Off-By: Kenneth Rowland [email protected]

kenrowland force-pushed the HPCC-32717 branch from 0112aa8 to 2535bc8 Compare October 14, 2024 14:57

kenrowland requested a review from ghalliday October 14, 2024 15:57

ghalliday approved these changes Oct 16, 2024

View reviewed changes

ghalliday merged commit d75da84 into hpcc-systems:master Oct 16, 2024
52 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HPCC-32717 Create basic Elastic sink component #19158

HPCC-32717 Create basic Elastic sink component #19158

kenrowland commented Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024

rpastrana left a comment

rpastrana Sep 26, 2024

kenrowland Sep 26, 2024

rpastrana Sep 26, 2024

kenrowland Sep 26, 2024

rpastrana Sep 26, 2024

kenrowland Sep 26, 2024

rpastrana Sep 26, 2024

kenrowland Sep 26, 2024

kenrowland commented Sep 26, 2024

ghalliday left a comment

ghalliday Sep 30, 2024

ghalliday Sep 30, 2024

ghalliday Sep 30, 2024

ghalliday Sep 30, 2024

kenrowland Sep 30, 2024

kenrowland Oct 1, 2024

ghalliday Sep 30, 2024

ghalliday Oct 3, 2024

kenrowland Oct 4, 2024

ghalliday Oct 9, 2024

		@@ -0,0 +1,59 @@
		/*##############################################################################
		HPCC SYSTEMS software Copyright (C) 2021 HPCC Systems®.

		@@ -0,0 +1,53 @@
		/*##############################################################################
		HPCC SYSTEMS software Copyright (C) 2021 HPCC Systems®.



		ElasticMetricSink::ElasticMetricSink(const char name, const IPropertyTree pSettingsTree) :
		PeriodicMetricSink(name, "file", pSettingsTree),

		@@ -0,0 +1,80 @@
		/*##############################################################################

HPCC-32717 Create basic Elastic sink component #19158

HPCC-32717 Create basic Elastic sink component #19158

Conversation

kenrowland commented Sep 26, 2024 • edited Loading

Type of change:

Checklist:

Smoketest:

Testing:

github-actions bot commented Sep 26, 2024

rpastrana left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenrowland commented Sep 26, 2024

ghalliday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kenrowland commented Sep 26, 2024 •

edited

Loading