[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-844082215 Happy to contribute, thank you for the guidance! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-840756785 @markap14 @markobean I just pushed a commit that I believe covers all outstanding items. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-839779931 > Thanks for the update @JonathanKessler . I do think that you addressed the concerns that I had in the first review. Looked over it again. There are a few places where the code should probably be cleaned up a bit (commented inline). Small changes to just make it easier to understand. > > There's also the potential for a NullPointerException that must be addressed before we can merge this. > > However, the stated goal of NIFI-8126 is to include a new metric about average queue duration, etc. in the stats history. This does not accomplish that. It updates the ConnectionStatus, but it doesn't give us anything in the UI to select the Average Queue Duration. The ConnectionStatusDescriptor needs to be updated to expose that information. I will make the requested changes. The ticket actually says to expose a total or an average. I went with the total, leaving it up to consuming systems to calculate the average. This matches other metrics available within ConnectionStatus such as inputBytes, queuedBytes, etc. I will look at adding that to the ConnectionStatusDescriptor. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-832020685 @markap14 Pinging again, can you please take another look? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-814925343 @markap14, just wanted to ping you to please take a second look when you have a moment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-795889476 I'm embarrassed by the flurry of commits today. I finally did a full build, test and checkstyle like I'm supposed to prior to pushing this last checkstyle fix. We should be good now. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi] JonathanKessler commented on pull request #4780: NIFI-8126 Include Total Queued Duration in ConnectionStatus metrics
JonathanKessler commented on pull request #4780: URL: https://github.com/apache/nifi/pull/4780#issuecomment-795522377 @markap14 I just pushed a commit that I believe addresses everything. It was a bit more involved than I anticipated so please let me know if I missed anything. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
