Skip to content

WIP: Provide zones and endpoints (ex local) stats via /v1/status #6499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

WIP: Provide zones and endpoints (ex local) stats via /v1/status #6499

wants to merge 3 commits into from

Conversation

Al2Klimov
Copy link
Member

No description provided.

@Al2Klimov Al2Klimov force-pushed the feature/cluster-stats branch 2 times, most recently from 379efe5 to 3f0f133 Compare August 1, 2018 13:25
@Al2Klimov Al2Klimov requested a review from dnsmichi August 1, 2018 14:22
@dnsmichi
Copy link
Contributor

dnsmichi commented Aug 2, 2018

Windows builds are failing with

"C:\projects\icinga2\build\PACKAGE.vcxproj" (default target) (1) ->
"C:\projects\icinga2\build\ALL_BUILD.vcxproj" (default target) (3) ->
"C:\projects\icinga2\build\test\boosttest-test-base.vcxproj" (default target) (8) ->
"C:\projects\icinga2\build\lib\icinga\icinga.vcxproj" (default target) (10) ->
"C:\projects\icinga2\build\lib\remote\remote.vcxproj" (default target) (12) ->
(ClCompile target) -> 
  c:\projects\icinga2\lib\remote\statsreporter.cpp(45): error C2280: 'std::atomic_flag::atomic_flag(const std::atomic_flag &)': attempting to reference a deleted function [C:\projects\icinga2\build\lib\remote\remote.vcxproj]

@Al2Klimov Al2Klimov force-pushed the feature/cluster-stats branch 4 times, most recently from 3d64536 to 30016a6 Compare August 2, 2018 12:38
@Al2Klimov
Copy link
Member Author 

$ curl .../v1/status/ClusterStats |python -mjson.tool

{
    "results": [
        {
            "name": "ClusterStats",
            "perfdata": [],
            "status": {
                "cluster": {
                    "node1": {
                        "endpoints": {
                            "node2": {
                                "connected": true,
                                "last_message_received": 1533213463.691841
                            }
                        },
                        "mtime": 1533213470.259481,
                        "uptime": 117.08613300323486,
                        "version": "v2.9.1-40-g30016a662"
                    },
                    "node2": {
                        "endpoints": {
                            "node1": {
                                "connected": true,
                                "last_message_received": 1533213453.690339
                            },
                            "node3": {
                                "connected": true,
                                "last_message_received": 1533213463.13135
                            }
                        },
                        "mtime": 1533213463.690132,
                        "uptime": 110.51873183250427,
                        "version": "v2.9.1-40-g30016a662"
                    },
                    "node3": {
                        "endpoints": {
                            "node2": {
                                "connected": true,
                                "last_message_received": 1533213453.69121
                            }
                        },
                        "mtime": 1533213463.129759,
                        "uptime": 110.31669497489929,
                        "version": "v2.9.1-40-g30016a662"
                    }
                }
            }
        }
    ]
}

dnsmichi
dnsmichi suggested changes Aug 3, 2018
View reviewed changes
lib/remote/statsreporter.cpp Outdated
{
Dictionary::Ptr message;

for (auto& zone : Zone::GetLocalZone()->GetAllParents()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will lead into many messages. When we talked about the possible algorithm here, I thought of sending the status to the immediate parent zone (and all endpoints), they update their local cache with what they receive from child zone, and then generate a new cluster message with aggregated data. That way the data may not be "live" but is always bound to "next hop, next hop".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right and this was an #ElephantOversight.

lib/remote/statsreporter.cpp Outdated
Dictionary::Ptr allStats (rawStats);

if (allStats) {
// For security reasons
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain that in this comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed it.

lib/remote/zone.cpp Outdated
{"messages_sent_per_second", (Array::Ptr)new Array},
{"messages_received_per_second", (Array::Ptr)new Array},
{"bytes_sent_per_second", (Array::Ptr)new Array},
{"bytes_received_per_second", (Array::Ptr)new Array}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the Array::Ptr casts and just use new Array().

lib/remote/zone.cpp
((Array::Ptr)endpointStats->Get("messages_sent_per_second"))->Add(endpoint->GetMessagesSentPerSecond());
((Array::Ptr)endpointStats->Get("messages_received_per_second"))->Add(endpoint->GetMessagesReceivedPerSecond());
((Array::Ptr)endpointStats->Get("bytes_sent_per_second"))->Add(endpoint->GetBytesSentPerSecond());
((Array::Ptr)endpointStats->Get("bytes_received_per_second"))->Add(endpoint->GetBytesReceivedPerSecond());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, the Array::Ptr casts are not necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid they are. If you don't believe me, try to build w/o them.

@Al2Klimov Al2Klimov force-pushed the feature/cluster-stats branch 3 times, most recently from 9f0735b to 86fe578 Compare August 6, 2018 07:23
@Al2Klimov Al2Klimov requested a review from dnsmichi August 6, 2018 07:44
@dnsmichi dnsmichi added the TBD To be defined - We aren't certain about this yet label Feb 11, 2019
@dnsmichi
Copy link
Contributor

@lippserd this was considered for IcingaDB a while ago, needs a decision on proceeding.

@dnsmichi dnsmichi mentioned this pull request Apr 10, 2019
Closed
@dnsmichi dnsmichi added area/api REST API area/distributed Distributed monitoring (master, satellites, clients) stalled Blocked or not relevant yet labels May 10, 2019
@dnsmichi dnsmichi mentioned this pull request Feb 5, 2020
Open
@Al2Klimov Al2Klimov added this to the 2.13.0 milestone Nov 23, 2020
@Al2Klimov Al2Klimov requested review from lippserd and removed request for dnsmichi December 14, 2020 18:10
@lippserd lippserd removed their request for review January 13, 2021 09:12
@julianbrost julianbrost modified the milestones: 2.13.0, 2.14.0 May 31, 2021
@Al2Klimov
Copy link
Member Author

@cla-bot check

@cla-bot cla-bot bot added the cla/signed label Aug 4, 2021
@Al2Klimov Al2Klimov added the enhancement New feature or request label Aug 10, 2021
@julianbrost julianbrost removed this from the 2.14.0 milestone Jan 12, 2023
@Al2Klimov Al2Klimov force-pushed the feature/cluster-stats branch 3 times, most recently from 16def46 to 663916c Compare May 22, 2023 11:18
@Al2Klimov Al2Klimov force-pushed the feature/cluster-stats branch from 663916c to 9f1541c Compare January 31, 2025 15:58
@Al2Klimov
Copy link
Member Author

Just trivially rebasing some PRs allowing to do so (regardless of their status!) to run GHA at all for the green checkmark. Please don't wonder. It's weekend after all soon, so no one gets blocked...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnsmichi dnsmichi dnsmichi requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/api REST API area/distributed Distributed monitoring (master, satellites, clients) cla/signed enhancement New feature or request stalled Blocked or not relevant yet TBD To be defined - We aren't certain about this yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Al2Klimov @dnsmichi @julianbrost