STORM-2153: New Metrics Reporting API

See JIRA for more information:
https://issues.apache.org/jira/browse/STORM-2153

@ptgoetz: thanks for your work on this.  But I may please request that you add some description about what you've actually done?  There is a lot of discussion in STORM-2153, and a ton of different topics are covered in that ticket, so it would be greatly beneficial if you could put some prose about what you've implemented.  Thanks!

I second @erikdw .
It would be better which part(s) this patch covers in STORM-2153. I can see various and numerous discussions from STORM-2153, and also various sub-issues. At a glance, this patch only covers several sub-issues, not exhaustive.
My first thought is, I guess this is not going to be a replacement with built-in metrics, right? While we have issue on metrics consumer side, but my observation (concern) was mainly focused to built-in metrics.

@erikdw Of course! This needs to be documented, and if accepted (or likely to be), I will do so. But if there's community consensus around NOT accepting this patch, I'd rather not waste the effort.
This only addresses the sub tasks of STORM-2153 that are marked "in progress."
The other issues, particularly the RocksDb, is likely off the table for now, since RocksDb is now considered category x (on a side note, a number of Apache projects like Kafka and flink are in for some hurt in that respect, since they will likely have to migrate grate off of RocksDb very shortly).
As far as your other question, aggregation is left to the destination metrics system. The individual metrics are namespaced with topology, host, port, etc. that uniquely identify each event down to the component/executor level. The namespace uses dot (".") notation to make it easy for tools to parse.
You can effectively disable this feature by not registering any reporters. The metrics objects will still be there, but there will be no reporters doing anything with them. If would be a fairly simple optimization to only register the metrics objects if there are one or more reporters registered. (IIRC this was one for f the concerns you raised in one of the discussion threads...)
@HeartSaVioR
See above for some of your questions.
This is an independent alternative to the existing metrics system. The goal is to make things simpler. I think a lot of users have struggled with storms built in metrics system, if they have approached it at all.

@ptgoetz
OK. Thanks for clarification. Makes sense.
Btw, I still wish that we address issues on built-in metrics and integrate newer thing to the UI, like what @abellina proposed in JIRA issue.
https://github.com/abellina/storm/blob/new_metrics_diagram/docs/new_metrics_phase_1.png
Do we plan to close all of sub-issues after merging this patch?
ps. I found what you said, RocksDB defining as Category X: https://issues.apache.org/jira/browse/LEGAL-303 . We've talked about RocksDB as metrics storage, and someone might consider that also as State storage. Unfortunate news.

@HeartSaVioR

Do we plan to close all of sub-issues after merging this patch?

No. I'm proposing a alternative. The RocksDb license issue is a blocker, so I'm proposing we move forward with just t his for now. Ultimately I'd like to see the built in metrics deprecated.

@ptgoetz Suppose we deprecate the built-in metrics, are they deprecated for public API or also Storm codebase? If we would want latter, we should have alternative way to collect metrics (maybe internal metrics consumer?) and represent to the UI.

This certainly is a bummer. I've been focusing on other projects so I have been out for a while. Sorry about this.
For the status, we have an interface that gets the metrics from codahale to nimbus and we have a rocksdb implementation. For the default out of the box metric, we could rocksdb out and populate the maps that nimbus uses for the UI, at least. Ideally, however, we'd replace rocksdb for another suitable store (suggestions?)
Along those lines, one of the interns at Oath has code working to use Hbase as a store. Perhaps that's not the "out of the box" solution, but that is using the same interface via Nimbus.

@abellina
Ah no problem. I can see experimental branch from your fork and also pull request for HBase storage.
Btw, from your status, nimbus is pulling (or some components are pushing, whatever) the metrics and UI can represent the metrics. Is my understanding correct?
Does the code (with RocksDB) work in production or in stage? Does it replace built-in metrics with metrics V2? Or does it also keep built-in metrics as it as? If it's former, I'd rather see alternatives for RocksDB which can make default metrics system "out of the box", and adopt that.
In-memory and snapshot (dump to file) approach (no H/A) may also be acceptable given that requirement for availability of metrics is not too strict.

@ptgoetz Suppose we deprecate the built-in metrics, are they deprecated for public API or also Storm codebase? If we would want latter, we should have alternative way to collect metrics (maybe internal metrics consumer?) and represent to the UI.

@HeartSaVioR
I'd definitely deprecate the public API. One approach we could take for potentially shipping metrics to nimbus would be to implement an additional reporter that pushes them to some (non-RocksDB ;) ) data store for consumption by Storm UI.

Just FYI: Facebook just removed PATENT file from RocksDB and make RocksDB be dual licenses (GPLv2 and Apache).
https://github.com/facebook/rocksdb/commit/3c327ac2d0fd50bbd82fe1f1af5de909dad769e6
Maybe I guess we will be good to go with RocksDB soon.

FYI on another news of RocksDB:
https://issues.apache.org/jira/browse/MARMOTTA-669?focusedCommentId=16094738&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16094738
Hi Sebastian - if you are using ROCKSDB in Marmotta prior to the latest two versions - where it has been changed to ALv2, then you need to remove it from your source tree unless it clearly is an "optional dependency" as defined https://www.apache.org/legal/resolved.html#optional

I read this as opposite way: the latest two versions of RocksDB are OK to be included to the source tree.

In this PR, I see that metrics V2 drops most of built-in metrics.
This PR maintains metrics V1 for built-in and V2 for user-defined and some of built-in, but in metrics V1 all of built-in metrics are provided to the metrics consumers, so users will lose some metrics after replacing metrics consumer to V2. Moreover I guess we would want to replace the old metrics (in storm internal) eventually.
For worker there's no more task-level metrics, only (worker, component)-level is available, and it only tracks emit, ack and fail. All metrics regarding latency are dropped. Netty metrics are dropped too. Some built-in metrics might not be able to be injected by users, so also need to consider about this when dropping metrics. Maybe would want to turn on and off recording part of metrics as Bobby suggested if they impact performance.
All of disruptor queue metrics are taken, but there are many disruptor queues in worker but metrics are worker-level, and it's gauge (not aggregation) so one of queue metrics would always overwrite.
So would like to see some explanations that which metrics are selected, how they're aggregated in worker, how decision has been made, etc.

@HeartSaVioR @revans2  I have read most of the tickets and issues of STORM-2153, there are many discussions about the metrics V2 and the now built-in metrics. But i didn't see any about the heartbeats part.
Actually the heartbeats now write too much things[ window stats of tasks together] to zookeeper[ or pacemaker ], and we report it for worker granularity which actually can be pre-aggregated by machine/supervisor.
With such heavy implementation, we have serious efficiency reduction when cluster grows to be large, like what i have described in JIRA task: https://issues.apache.org/jira/browse/STORM-2693.
So i wanna promote the heartbeats part, which can be done individually apart the metrics V2 thing, this is what i will design the heartbeats to be:

After the promotion, we do not need to read the heartbeats from zookeeper any more when scheduling which will cost much time. But opening Ui will still be slow[ not a big deal anyway ].

@arunmahadevan

Currently this patch addresses registering custom metrics and sending the metrics out via reporters and does not send any metrics to nimbus right? Why is disruptor metrics handled as a part of this?

No, this does not send any metrics to nimbus. Disruptor metrics are included because many users would like to track those metrics (and they aren't available via Storm UI/REST).

There will be one thread per reporter per worker which sends out the metrics to the destination? Just thinking if it would overwhelm the destination and would need some local aggregation and have the supervisor report the metrics.

The idea here was to purposely avoid attempting aggregation, since in some cases it's impossible (e.g. there's no way to aggregate histograms). Instead, any aggregation is assumed to take place in the destination system (Grafana, etc.).

@HeartSaVioR

We need to document how to use new metrics and its reporter, and also need to have patch for master branch (maybe with removal of metrics V1 public API).

As I mentioned elsewhere, I want to hold off on porting this to the master branch until all comments are addressed and this patch is in a mergeable state. That way we can avoid porting small changes between the two branches.
As far as deprecating/removing the metrics v1 API, I'd hold off until there is a consensus that this approach is a suitable replacement for most use cases.

@revans2, @HeartSaVioR, @arunmahadevan Addressed review comments.
Also note that this is a feature branch, so it's open to pull requests from anyone. If there's anything you'd like to see that I haven't addressed (e.g. adding fields to metrics names) feel free to suggest.
Re: Documentation and port to master: Once this is merged I plan to add documentation and port to master. I can file a JIRA for those if desired.

@danny0405 @abellina actually has a patch to do what you were asking about.  To push metrics to nimbus by way of the supervisor.  It is obviously based off of this patch, so a lot of this is waiting for this one to be done before putting that one up.

@revans2 I agree with you regarding getting the metrics naming correct. In fact I think it's one of the most important aspects. I'd like to get as much input/collaboration from others as possible to make sure we get it right from the start.

So then perhaps I can summarize what we have right now for the naming.
"storm.topology.%s.%s.%s-%s", getStormId(), getThisComponentId(), getThisWorkerPort(), name
Where getStormId() is the topology id which should match ${name}-${counter}-${time-stamp} where name matches ^[^/.:\]+$
getThisComponentId() which as far as I can tell has no limits on what the name can be.
getThisWorkerPort() which is just an integer for the port this worker is listening on (no host name that it is a part of)
and name which has no limitations on it either.
Dropwizard (the underlying metrics system) requires that names be unique within a metrics registry, but places no other limitations except that.

So to parse these pieces of information back out I would need to do something like remove "storm.topology." from the beginning.
Then everything up to the next "." would be the topology-ID because "." is not allowed in the name.  I could go even further and parse out the topology name too.
Now I am kind of in trouble, because I have two different fields that can be anything on both ends of the name.  So my best bet is to look for something that looks like \.(\d+)- and hope that neither the component name nor the metric name happens to have used something that matches that pattern.
My concern here is that most of what we are truly encoding in the name is metadata.  All of the examples that graphite and ganglia show use the name as a hierarchy to make finding the metric I want simpler, i.e. 'system.memory.memory_allocated', or perhaps subdividing the metrics by application, 'app1.memory.memory_allocated'.  Metadata like the host name, port, component, etc, are really supported through tags.  The collectors are not attempting to pull out the metadata and then turn it back into tags.  So for example lets start with a simple use case. I want to know how many tuples component B has emitted.  Currently that would require me to combine N separate metrics together, one per port.
storm.topology.MYTOPO-1-1000.B.6700-emitted
storm.topology.MYTOPO-1-1000.B.6701-emitted
...
storm.topology.MYTOPO-1-1000.B.N-emitted
and then rewrite if I launch my topology again I now have to rewrite all of my queries to match.  I personally think everyone would be a lot happier if we could find a good way to translate these into tags.
Additionally the built in metrics that mostly @abellina, and me a tiny bit, have been working on wants these to be metadata.  Simply because we cannot reproduce what is happening on the UI in any other clean way.  I am fine with us using the metric name to store some of this metadata (sadly drop-wizard gives us no other option), but we really need to parse it out before sending it on.  I am not sure how simple that would be, seeing how they already come with built in reporters which was a lot of what we wanted them for.  If it is not going to be simple to do, then perhaps dropwizard was the wrong choice for a metrics library.

@revans2 Added rudimentary sanity check validation for metrics reporters configs. Because reporter implementations may want to have their own custom config keys, we can't really cover everything. I think we'll have to settle on basic sanity checks + reporters documenting the custom configuration fields they expose.
I'll think more about metrics naming and metadata. Like I said, feedback, opinions and pull requests against this branch are more than welcome.

@ptgoetz I think we can fix most of the issues simply by restricting what name a metric can have (i.e. no : allowed and then we use : as a deliminator).  That will give us a guarantee on being able to parse it.  Once we can parse it hopefully we can look at updating the other plugins to make them be able to parse the name and turn it into a smaller name with tags.

I also want to make sure that we take the performance impact of this patch into consideration.  With the old metrics system we are sub-sampling the metrics for performance reasons.  With this patch we are not.  When I run ThroughputVsLatency, not perfect I know.  With this patch the maximum throughput I am able to get is about 48,000 sentences per second.  Without this patch I can do about 60,000 reliably.  This is a 20% drop in maximum throughput.  Note that all I did was adjust the target throughput and set max spout pending to 500.

This patch adds new types of metrics on top of current metrics, so performance hit is expected. (20% drop looks like not small indeed.) We may want to consider not supporting smooth transition because of this.
Even we don't support smooth transition, we may want to comment all the old metrics and re-run benchmark to measure real performance hit.

@HeartSaVioR I agree that if we cannot find a better way we may not want to support a smooth transition for the metrics, but that also means that we cannot put it into 1.x.
But I think the underlying problem is actually in our choice of metric.  We are using a meter everywhere, and looking at the code a meter does not appear that cheap.  If all we want/need is a running count, and not various rates we might want to look at using a Counter instead.

@revans2 I need to recheck which thing (meter or counter) they are using, but JStorm also doesn’t sample metrics other than histogram. Unless they did custom optimization, IMHO it should be fast enough for us too.

@revans2 You may have to jog your memory ;), but do you recall the parameters you used for ThroughputVsLatency?
I just ran ThroughputVsLatency (with defaults) on an isolated machine (not a VM) and saw no significant differences in performance. I tested with the latest 1.x-branch and the latest metrics_v2 (up merged agains 1.x-branch).

@ptgoetz I think I just ran it with default options on my MBP.  But I will try and reproduce it again.

@ptgoetz @revans2
Here’s my result on performance test:
CLI option:
org.apache.storm.starter.ThroughputVsLatency 50000 -c topology.max.spout.pending=5000
so that utilizes 4 workers by default.

1.2.0-SNAPSHOT

uptime:  210 acked: 1,503,840 acked/sec:  50,128.00 failed:        0 99%:      34,734,079 99.9%:      48,103,423 min:       6,541,312 max:      78,381,055 mean:   13,820,460.15 stddev:    4,842,731.32 user:    173,260 sys:     31,980 gc:      2,583 mem:     669.23
uptime:  240 acked: 1,503,740 acked/sec:  50,124.67 failed:        0 99%:      33,456,127 99.9%:      53,346,303 min:       6,479,872 max:      80,412,671 mean:   13,642,800.22 stddev:    4,618,847.37 user:    172,510 sys:     31,690 gc:      2,513 mem:     614.37
uptime:  270 acked: 1,503,280 acked/sec:  50,109.33 failed:        0 99%:      33,013,759 99.9%:      48,201,727 min:       6,574,080 max:      78,315,519 mean:   13,656,544.79 stddev:    4,527,513.03 user:    173,000 sys:     31,370 gc:      2,513 mem:     740.95

uptime:  211 acked: 1,503,700 acked/sec:  48,506.45 failed:        0 99%:      35,061,759 99.9%:      53,444,607 min:       6,516,736 max:      84,410,367 mean:   14,023,839.49 stddev:    4,968,132.86 user:    173,490 sys:     33,240 gc:      2,453 mem:   1,042.78
uptime:  241 acked: 1,503,920 acked/sec:  50,130.67 failed:        0 99%:      32,882,687 99.9%:      53,968,895 min:       6,574,080 max:      81,133,567 mean:   13,700,645.27 stddev:    4,592,749.72 user:    173,260 sys:     33,190 gc:      2,465 mem:   1,007.66
uptime:  271 acked: 1,503,260 acked/sec:  50,108.67 failed:        0 99%:      33,275,903 99.9%:      56,262,655 min:       6,582,272 max:      81,199,103 mean:   13,710,314.71 stddev:    4,676,515.80 user:    173,910 sys:     32,430 gc:      2,440 mem:   1,065.66

Metrics V2

uptime:  211 acked: 1,503,580 acked/sec:  50,119.33 failed:        0 99%:      40,861,695 99.9%:      62,783,487 min:       6,496,256 max:     106,692,607 mean:   14,696,942.76 stddev:    6,041,492.37 user:    187,800 sys:     32,170 gc:      2,646 mem:     779.90
uptime:  241 acked: 1,541,060 acked/sec:  51,368.67 failed:        0 99%:      41,779,199 99.9%:      70,778,879 min:       6,639,616 max:     113,115,135 mean:   14,872,133.61 stddev:    6,435,291.03 user:    219,910 sys:     36,630 gc:      3,115 mem:     875.09
uptime:  271 acked: 1,503,780 acked/sec:  50,126.00 failed:        0 99%:      41,189,375 99.9%:      63,733,759 min:       6,529,024 max:     104,267,775 mean:   14,738,586.49 stddev:    6,153,017.22 user:    188,950 sys:     31,950 gc:      2,815 mem:     891.73

uptime:  210 acked: 1,503,520 acked/sec:  50,117.33 failed:        0 99%:      47,120,383 99.9%:      89,391,103 min:       6,520,832 max:     142,999,551 mean:   15,073,527.31 stddev:    7,453,484.52 user:    186,330 sys:     33,150 gc:      2,808 mem:   1,096.18
uptime:  241 acked: 1,540,960 acked/sec:  49,708.39 failed:        0 99%:      36,012,031 99.9%:      51,281,919 min:       6,688,768 max:      74,055,679 mean:   14,267,705.37 stddev:    4,971,492.31 user:    186,600 sys:     33,710 gc:      2,462 mem:   1,192.20
uptime:  271 acked: 1,541,440 acked/sec:  51,381.33 failed:        0 99%:      40,075,263 99.9%:      55,181,311 min:       6,725,632 max:      76,021,759 mean:   14,850,073.94 stddev:    5,832,959.05 user:    187,380 sys:     32,640 gc:      2,577 mem:   1,036.03

The value of 99% and 99.9% latency seems indicating the performance hit. Below is the values from 99.9% latency:

1.2.0-SNAPSHOT
avg(48, 53, 48) => 49.6
avg(53, 53, 56) => 54

metrics_v2
avg(62, 70, 63) => 65
avg(89, 51, 55) => 65

We may could do another perspective of test: putting higher rate and see which one reaches the limit first.

We really need hardware and environment  information. I'd also argue that tests should be run headless. I've seen some benchmarks vary greatly on a MBP depending on what you're doing at the time. Something as mundane as checking email, etc. can affect benchmark results.
sigar might be an easy target (since it's included) to get some of that info.

Here's my machine spec used for performance test:
java version "1.8.0_144"
Java(TM) SE Runtime Environment (build 1.8.0_144-b01)
Java HotSpot(TM) 64-Bit Server VM (build 25.144-b01, mixed mode)
OS: Ubuntu 17.10
CPU: AMD Ryzen 5 1600 3.2Ghz 6 core (with hyper-thread = 12 logical cores)
RAM: Samsung DDR4 32G 19200
SSD: Samsung 850 Evo
I have another dev. machine and I only use the env. for testing at that time. It only runs some background processes, but CPU usage was around 1 % before running the tests all the time.

You can scroll the result to see rightmost side which shows CPU, Memory, GC as well.

@HeartSaVioR I know. ;)
I'm thinking more in terms of hardware profile and what other processes are running.

OK. While I understand what you're saying, that's not easy for individual to get it, cause we individual don't want to afford completely idle machine and use for only performance test. I have been spining up AWS VM with "dedicated" option to set up clear env. and install mandatory services, and go ahead. Unfortunately I used all of my AWS coupons and have to rely on my PC which is closely idle. ;)
One approach is running performance tests several times, and drop outliers and take average or just see trends. If the performance of A is always or most of the times greater/lower than B with enough trials, I'd rather believe the result. (would 5 times be sufficient?)

Here are my numbers, and no I didn't do it headless but I was not doing anything on the laptop at the time it was running except looking at Activity Monitor to be sure nothing else was using too much CPU.
The setup is
MacBook Pro (Retina, 15-inch, Mid 2015)
2.8 GHz Intel Core i7
16 GB 1600 MHz DDR3
macOS 10.12.6
java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)
The versions tested are 1.2.0-SNAPSHOT (b24f5d87cbcd4faeed651e45a8e673db52469a46)
Metrics V2 (78d3de44cf0705efd199fb64b7ef092e23c3285b) which is 1.2.0-SNAPSHOT + 99bcf68 merged in.
The Numbers:
1.2.0-SNAPSHOT
./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 50000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked:   602,660 acked/sec:  20,088.67 failed:        0 99%:     741,343,231 99.9%:     757,596,159 min:       3,639,296 max:     768,081,919 mean:   95,373,435.91 stddev:  191,784,964.85 user:     71,070 sys:      4,565 gc:        247 mem:     109.46
uptime:   60 acked: 1,504,480 acked/sec:  50,149.33 failed:        0 99%:      13,320,191 99.9%:      15,794,175 min:       3,467,264 max:      30,490,623 mean:    9,679,685.54 stddev:    1,394,068.49 user:     72,565 sys:      6,299 gc:        728 mem:      43.30
uptime:   90 acked: 1,510,500 acked/sec:  50,350.00 failed:        0 99%:      13,279,231 99.9%:      14,704,639 min:       3,352,576 max:      17,612,799 mean:    9,650,125.87 stddev:    1,358,879.30 user:     72,788 sys:      6,360 gc:        732 mem:      73.51
uptime:  120 acked: 1,503,720 acked/sec:  50,124.00 failed:        0 99%:      13,352,959 99.9%:      14,745,599 min:       3,356,672 max:      16,973,823 mean:    9,683,344.13 stddev:    1,361,582.42 user:     72,855 sys:      6,239 gc:        740 mem:     101.27
uptime:  150 acked: 1,654,820 acked/sec:  55,160.67 failed:        0 99%:      13,369,343 99.9%:      14,811,135 min:       3,473,408 max:      16,736,255 mean:    9,695,874.65 stddev:    1,363,167.65 user:     72,671 sys:      6,327 gc:        741 mem:     127.25
uptime:  180 acked: 1,503,740 acked/sec:  50,124.67 failed:        0 99%:      13,377,535 99.9%:      14,811,135 min:       3,448,832 max:      17,711,103 mean:    9,682,345.18 stddev:    1,370,041.64 user:     72,848 sys:      6,280 gc:        756 mem:      41.83

./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 75000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked:   882,420 acked/sec:  29,414.00 failed:        0 99%:     746,586,111 99.9%:     763,363,327 min:       3,678,208 max:     768,081,919 mean:  202,478,108.44 stddev:  266,947,569.24 user:     50,763 sys:      2,752 gc:          0 mem:      38.93
uptime:   60 acked: 2,497,720 acked/sec:  83,257.33 failed:        0 99%:     213,123,071 99.9%:     256,245,759 min:       3,672,064 max:     271,581,183 mean:   46,314,967.38 stddev:   51,559,780.69 user:    118,361 sys:      7,777 gc:      1,322 mem:      85.28
uptime:   90 acked: 2,260,200 acked/sec:  75,340.00 failed:        0 99%:     129,826,815 99.9%:     152,698,879 min:       3,659,776 max:     160,432,127 mean:   29,179,878.90 stddev:   29,586,943.44 user:    110,350 sys:      7,583 gc:      1,323 mem:      30.47
uptime:  120 acked: 2,255,680 acked/sec:  75,189.33 failed:        0 99%:     148,504,575 99.9%:     179,306,495 min:       3,700,736 max:     187,301,887 mean:   16,040,979.89 stddev:   23,589,666.52 user:    103,839 sys:      7,031 gc:      1,174 mem:      51.50
uptime:  150 acked: 2,255,180 acked/sec:  75,172.67 failed:        0 99%:      81,002,495 99.9%:     117,309,439 min:       3,571,712 max:     123,994,111 mean:   10,956,757.98 stddev:   11,465,433.32 user:    100,497 sys:      6,960 gc:      1,136 mem:      90.77
uptime:  180 acked: 2,256,380 acked/sec:  75,212.67 failed:        0 99%:      35,192,831 99.9%:      49,446,911 min:       3,608,576 max:      55,672,831 mean:    9,647,021.99 stddev:    4,081,135.34 user:    100,516 sys:      6,771 gc:      1,141 mem:     110.73

./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 100000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked: 1,023,620 acked/sec:  34,120.67 failed:        0 99%:   1,427,111,935 99.9%:   1,442,840,575 min:     141,033,472 max:   1,448,083,455 mean: 1,205,785,740.36 stddev:  167,013,014.66 user:     56,266 sys:      2,615 gc:          0 mem:      53.83
uptime:   60 acked: 2,820,000 acked/sec:  94,000.00 failed:        0 99%:   4,177,526,783 99.9%:   4,320,133,119 min:   1,091,567,616 max:   4,353,687,551 mean: 2,617,160,518.91 stddev:  618,922,278.84 user:    127,359 sys:      7,415 gc:      1,565 mem:      88.42
uptime:   90 acked: 2,665,280 acked/sec:  88,842.67 failed:        0 99%:   7,541,358,591 99.9%:   7,918,845,951 min:   2,944,401,408 max:   7,948,206,079 mean: 4,756,939,536.76 stddev: 1,117,314,866.53 user:    128,113 sys:      7,796 gc:      1,593 mem:     115.03
uptime:  120 acked: 2,919,360 acked/sec:  97,312.00 failed:        0 99%:  12,683,575,295 99.9%:  12,784,238,591 min:   5,440,012,288 max:  12,792,627,199 mean: 9,308,123,106.98 stddev: 1,615,564,564.30 user:    121,475 sys:      7,534 gc:      1,567 mem:      47.30
uptime:  150 acked: 2,877,200 acked/sec:  95,906.67 failed:        0 99%:  14,646,509,567 99.9%:  14,763,950,079 min:   9,302,966,272 max:  14,789,115,903 mean: 11,379,620,943.33 stddev: 1,593,183,614.74 user:    133,995 sys:      7,393 gc:      1,616 mem:     133.84
uptime:  181 acked: 2,859,340 acked/sec:  92,236.77 failed:        0 99%:  16,366,174,207 99.9%:  16,424,894,463 min:  10,116,661,248 max:  16,450,060,287 mean: 12,700,704,569.76 stddev: 2,014,006,250.70 user:    129,856 sys:      7,147 gc:      1,592 mem:      90.25

Metrics V2:
./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 50000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked:   751,560 acked/sec:  25,052.00 failed:        0 99%:     499,908,607 99.9%:     505,675,775 min:       4,108,288 max:     509,870,079 mean:  107,075,292.72 stddev:  173,303,090.76 user:     47,136 sys:      2,570 gc:          0 mem:      65.99
uptime:   60 acked: 1,505,040 acked/sec:  50,168.00 failed:        0 99%:      86,835,199 99.9%:     127,336,447 min:       3,575,808 max:     134,873,087 mean:   11,308,860.12 stddev:   11,073,472.42 user:     87,512 sys:      6,176 gc:        763 mem:      67.76
uptime:   90 acked: 1,503,580 acked/sec:  50,119.33 failed:        0 99%:      13,901,823 99.9%:      15,450,111 min:       3,684,352 max:      18,726,911 mean:    9,930,800.24 stddev:    1,361,960.54 user:     86,289 sys:      6,234 gc:        768 mem:      93.17
uptime:  120 acked: 1,504,200 acked/sec:  50,140.00 failed:        0 99%:      39,190,527 99.9%:      92,864,511 min:       3,645,440 max:     100,859,903 mean:   10,615,784.00 stddev:    6,838,619.84 user:     86,522 sys:      6,275 gc:        787 mem:     114.78
uptime:  150 acked: 1,503,880 acked/sec:  50,129.33 failed:        0 99%:      13,885,439 99.9%:      15,302,655 min:       3,964,928 max:      19,693,567 mean:    9,949,763.37 stddev:    1,350,149.76 user:     86,026 sys:      6,226 gc:        789 mem:     136.70
uptime:  180 acked: 1,503,840 acked/sec:  50,128.00 failed:        0 99%:      13,967,359 99.9%:      15,515,647 min:       3,670,016 max:      19,316,735 mean:    9,921,255.02 stddev:    1,380,147.00 user:     85,840 sys:      6,307 gc:        814 mem:      61.50

./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 75000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked:   810,700 acked/sec:  27,023.33 failed:        0 99%:   1,469,054,975 99.9%:   1,484,783,615 min:     110,428,160 max:   1,490,026,495 mean: 1,103,688,559.63 stddev:  218,353,127.14 user:     59,994 sys:      2,680 gc:          0 mem:     110.42
uptime:   60 acked: 1,580,040 acked/sec:  52,668.00 failed:        0 99%:   2,017,460,223 99.9%:   2,138,046,463 min:     305,135,616 max:   2,151,677,951 mean: 1,271,180,309.48 stddev:  349,131,056.74 user:    130,862 sys:      7,058 gc:      1,521 mem:      80.61
uptime:   90 acked: 2,279,900 acked/sec:  75,996.67 failed:        0 99%:   2,208,301,055 99.9%:   2,262,827,007 min:      13,189,120 max:   2,271,215,615 mean:  876,751,968.81 stddev:  666,332,632.22 user:    128,889 sys:      7,008 gc:      1,531 mem:      69.38
uptime:  120 acked: 2,305,340 acked/sec:  76,844.67 failed:        0 99%:   2,138,046,463 99.9%:   2,298,478,591 min:       4,882,432 max:   2,317,352,959 mean:  440,761,887.25 stddev:  592,545,096.04 user:    130,032 sys:      6,988 gc:      1,491 mem:      84.19
uptime:  151 acked: 2,277,980 acked/sec:  73,483.23 failed:        0 99%:      60,391,423 99.9%:      72,155,135 min:       3,788,800 max:      80,216,063 mean:   21,421,869.58 stddev:   13,674,045.52 user:    123,179 sys:      6,837 gc:      1,367 mem:     128.93
uptime:  181 acked: 2,254,200 acked/sec:  75,140.00 failed:        0 99%:     214,433,791 99.9%:     238,551,039 min:       4,816,896 max:     247,332,863 mean:   52,724,728.68 stddev:   43,440,715.92 user:    124,503 sys:      6,916 gc:      1,454 mem:      46.17

./bin/storm jar ../../../examples/storm-starter/target/storm-starter-1.2.0-SNAPSHOT.jar org.apache.storm.starter.ThroughputVsLatency 100000 1 3 -c topology.max.spout.pending=500
uptime:   30 acked: 1,123,900 acked/sec:  37,463.33 failed:        0 99%:   2,592,079,871 99.9%:   2,600,468,479 min:      73,465,856 max:   2,652,897,279 mean: 1,690,311,396.11 stddev:  480,273,985.78 user:     57,811 sys:      2,666 gc:          0 mem:      61.51
uptime:   60 acked: 2,262,680 acked/sec:  75,422.67 failed:        0 99%:  10,208,935,935 99.9%:  10,536,091,647 min:   2,415,919,104 max:  10,594,811,903 mean: 6,285,873,303.48 stddev: 2,142,972,667.99 user:    127,730 sys:      6,723 gc:      1,482 mem:     117.42
uptime:   90 acked: 2,313,860 acked/sec:  77,128.67 failed:        0 99%:  17,834,180,607 99.9%:  18,052,284,415 min:   9,596,567,552 max:  18,102,616,063 mean: 13,442,454,586.16 stddev: 1,991,956,204.06 user:    131,847 sys:      7,115 gc:      1,534 mem:      70.55
uptime:  120 acked: 2,334,940 acked/sec:  77,831.33 failed:        0 99%:  24,964,497,407 99.9%:  25,316,818,943 min:  16,315,842,560 max:  25,350,373,375 mean: 20,534,292,592.51 stddev: 2,142,432,639.60 user:    130,283 sys:      6,860 gc:      1,499 mem:      73.80
uptime:  150 acked: 2,355,160 acked/sec:  78,505.33 failed:        0 99%:  32,061,259,775 99.9%:  32,413,581,311 min:  22,749,904,896 max:  32,463,912,959 mean: 27,008,051,127.76 stddev: 2,183,882,755.64 user:    129,694 sys:      6,936 gc:      1,496 mem:      68.42
uptime:  180 acked: 2,346,800 acked/sec:  78,226.67 failed:        0 99%:  38,889,586,687 99.9%:  39,258,685,439 min:  29,024,583,680 max:  39,292,239,871 mean: 33,518,848,499.25 stddev: 2,297,910,092.43 user:    131,058 sys:      6,849 gc:      1,514 mem:      71.15

At 50,000 sentences per second on a single worker both can keep up, but the mean latency appears to be about 200 to 300 microseconds slower 9.6 ms vs 9.9 ms.  The bigger issue in my mind is the CPU time as it went up from about 79 seconds (max 30 second period) every 30 seconds to 92 seconds (min 30 second period after stabilizing). That was at least a 16% increase in CPU usage.
At 75,000 sentences per second metrics v2 was just able to keep up with that throughput while 1.2.0-SNAPSHOT could handle it easily. I will not compare the latency because metrics v2 didn't stabilize enough for me to feel good reporting the numbers, but the CPU time went up from 118 seconds every 30 seconds to 130 seconds.  That was an increase of at least 9%.
When I tried to do 100,000 sentences per second neither of the two could keep up, but 1.2.0-SNAPSHOT hit a maximum of 97k while metrics V2 hit a maximum of 78k or 20% less maximum throughput.  Only here did we see the CPU usage be lower for metrics V2 with 134 seconds vs 141 for 1.2.0-SNAPSHOT, but if we take into account the actual throughput and look at CPU seconds needed to process 1k sentences per second it is 1.7 for metrics v2 vs 1.5 for 1.2.0-SNAPSHOT.
If you need me to run the numbers again on different hardware I can, but it may take me some more time to get access to bare metal as I find VMs produce less reliable numbers than my laptop does.
I am not saying that these changes are a blocker for the code going in, but I really want to understand what is happening here

Also looking at the numbers from @HeartSaVioR.  They also show the CPU being higher, but not nearly as much as mine do 219 seconds for V2 vs 206 seconds for 1.2.0 or about a 6% difference.
Again I really just want to understand where the extra load is coming from so we can understand how to mitigate it and/or better reason about what will happen when there are more metrics being used.

@revans2 Thanks for the update.
My first instinct is that it’s usage of Meter on the critical path. Adding anything that “does something” there is going to add some level of overhead. I’ll do some (micro)benchmarking and experiments to find out. I think there will be a number of mitigation options.

I've tried out replacing meters to counters: here is the branch
https://github.com/HeartSaVioR/storm/tree/metrics_v2_replace_meters_to_counters

metrics_v2_replace_meters_to_counters

uptime:  210 acked: 1,504,280 acked/sec:  50,142.67 failed:        0 99%:      35,192,831 99.9%:      52,658,175 min:       6,561,792 max:      97,648,639 mean:   14,233,952.18 stddev:    4,975,946.97 user:    192,210 sys:     36,820 gc:      2,604 mem:     811.39
uptime:  240 acked: 1,503,860 acked/sec:  50,128.67 failed:        0 99%:      53,510,143 99.9%:      88,276,991 min:       6,643,712 max:     128,581,631 mean:   15,472,482.77 stddev:    8,210,141.23 user:    178,240 sys:     34,680 gc:      2,605 mem:     889.04
uptime:  271 acked: 1,578,380 acked/sec:  50,915.48 failed:        0 99%:      38,436,863 99.9%:      57,049,087 min:       6,717,440 max:      92,274,687 mean:   14,373,815.12 stddev:    5,487,140.26 user:    196,260 sys:     37,490 gc:      2,780 mem:     916.19

uptime:  210 acked: 1,541,100 acked/sec:  51,370.00 failed:        0 99%:      37,486,591 99.9%:      57,409,535 min:       6,664,192 max:      75,169,791 mean:   14,192,844.52 stddev:    5,369,622.19 user:    179,110 sys:     33,210 gc:      2,571 mem:     677.76
uptime:  240 acked: 1,541,240 acked/sec:  51,374.67 failed:        0 99%:      38,404,095 99.9%:      56,819,711 min:       6,651,904 max:      85,590,015 mean:   14,508,982.42 stddev:    5,551,974.72 user:    178,550 sys:     34,130 gc:      2,614 mem:     740.89
uptime:  270 acked: 1,503,300 acked/sec:  50,110.00 failed:        0 99%:      37,093,375 99.9%:      59,506,687 min:       6,537,216 max:      92,078,079 mean:   14,124,510.04 stddev:    5,286,365.69 user:    177,730 sys:     33,530 gc:      2,698 mem:     755.77

uptime:  211 acked: 1,503,700 acked/sec:  50,123.33 failed:        0 99%:      32,571,391 99.9%:      49,938,431 min:       6,483,968 max:      81,592,319 mean:   13,690,758.91 stddev:    4,494,703.26 user:    176,600 sys:     34,130 gc:      2,503 mem:     893.43
uptime:  241 acked: 1,578,620 acked/sec:  52,620.67 failed:        0 99%:      39,649,279 99.9%:      63,143,935 min:       6,541,312 max:      98,631,679 mean:   14,144,096.06 stddev:    5,696,236.12 user:    177,100 sys:     33,880 gc:      2,671 mem:     889.88
uptime:  271 acked: 1,540,000 acked/sec:  51,333.33 failed:        0 99%:      40,632,319 99.9%:      66,912,255 min:       6,782,976 max:     112,984,063 mean:   14,480,762.35 stddev:    6,064,099.29 user:    180,220 sys:     33,230 gc:      2,743 mem:     832.76

1.2.0-SNAPSHOT

uptime:  211 acked: 1,503,480 acked/sec:  50,116.00 failed:        0 99%:      35,422,207 99.9%:      52,330,495 min:       6,602,752 max:      88,211,455 mean:   14,071,994.11 stddev:    4,935,845.81 user:    174,870 sys:     34,440 gc:      2,531 mem:     888.99
uptime:  241 acked: 1,503,540 acked/sec:  50,118.00 failed:        0 99%:      36,208,639 99.9%:      52,527,103 min:       6,619,136 max:      95,617,023 mean:   14,146,908.46 stddev:    5,030,666.13 user:    176,370 sys:     34,290 gc:      2,500 mem:     961.91
uptime:  271 acked: 1,616,180 acked/sec:  53,872.67 failed:        0 99%:      34,111,487 99.9%:      52,133,887 min:       6,529,024 max:      83,099,647 mean:   14,007,535.10 stddev:    4,706,000.17 user:    175,580 sys:     33,900 gc:      2,458 mem:   1,039.27

uptime:  210 acked: 1,578,340 acked/sec:  52,611.33 failed:        0 99%:      34,668,543 99.9%:      51,249,151 min:       6,664,192 max:      82,575,359 mean:   13,965,232.22 stddev:    4,855,563.94 user:    175,000 sys:     33,820 gc:      2,415 mem:     902.57
uptime:  240 acked: 1,541,200 acked/sec:  51,373.33 failed:        0 99%:      34,144,255 99.9%:      51,478,527 min:       5,931,008 max:      80,150,527 mean:   13,730,336.55 stddev:    4,719,024.27 user:    175,660 sys:     33,430 gc:      2,403 mem:     825.49
uptime:  270 acked: 1,504,080 acked/sec:  50,136.00 failed:        0 99%:      34,340,863 99.9%:      51,019,775 min:       6,594,560 max:      79,167,487 mean:   13,756,017.32 stddev:    4,619,909.40 user:    175,510 sys:     34,050 gc:      2,366 mem:     819.57

uptime:  211 acked: 1,541,080 acked/sec:  51,369.33 failed:        0 99%:      29,130,751 99.9%:      48,037,887 min:       6,631,424 max:      87,162,879 mean:   13,285,878.61 stddev:    3,908,718.02 user:    172,650 sys:     33,080 gc:      2,203 mem:     949.73
uptime:  241 acked: 1,503,620 acked/sec:  50,120.67 failed:        0 99%:      38,830,079 99.9%:      58,392,575 min:       6,602,752 max:      92,340,223 mean:   13,945,274.39 stddev:    5,359,104.05 user:    173,580 sys:     32,890 gc:      2,295 mem:   1,078.11
uptime:  271 acked: 1,503,400 acked/sec:  50,113.33 failed:        0 99%:      31,293,439 99.9%:      45,219,839 min:       6,688,768 max:      73,269,247 mean:   13,384,094.26 stddev:    4,055,526.66 user:    173,060 sys:     33,270 gc:      2,193 mem:   1,070.78

So the difference on CPU usage and latency is reduced, whereas there's still gap between twos. That's what we can imagine, so please do some performance tests and see the gaps, and discuss whether the gap is tolerable.

I did a quick analysis of the latency vs CPU usage for the branch from @HeartSaVioR and this branch based off of the test results posted by @HeartSaVioR (just to get more of an apples to apples comparison, but my test results look similar).
It looks like metrics_v2_replace_meters_to_counters had an increased latency over 1.2.0-SNAPSHOT by about 2.1% to 4.0% (depending on if I do a mean vs a median of the reported mean latency values)  Similarly for CPU usage it was between 1.7% to 3.8% more CPU usage.
When using the normal metrics v2 it was a 7.2% to 8.0% increase in latency and 7.0% to 10.1% increase in CPU.  So this is a huge step in the right direction.
I really would love to see an overhead of < 1%, but I could probably live with these numbers.
Would it be possible to try an experiment where we also do random sub-sampling with these metrics?  I know it is a giant confusion for people to not have an exact number for metrics, but I would be curious to see what difference if any it would make so we know what the issue is.

I created another branch for sampling:
https://github.com/HeartSaVioR/storm/tree/metrics_v2_replace_meters_to_counters_with_sampling

metrics_v2_replace_meters_to_counters_with_sampling

uptime:  211 acked: 1,578,860 acked/sec:  52,628.67 failed:        0 99%:      32,849,919 99.9%:      48,136,191 min:       6,545,408 max:      81,461,247 mean:   13,719,507.76 stddev:    4,401,893.83 user:    174,310 sys:     34,720 gc:      2,159 mem:     777.26
uptime:  241 acked: 1,541,220 acked/sec:  51,374.00 failed:        0 99%:      34,078,719 99.9%:      79,495,167 min:       5,988,352 max:     142,737,407 mean:   13,692,679.51 stddev:    5,447,765.36 user:    175,140 sys:     33,960 gc:      2,189 mem:     864.25
uptime:  271 acked: 1,503,360 acked/sec:  50,112.00 failed:        0 99%:      31,670,271 99.9%:      50,724,863 min:       6,541,312 max:      86,638,591 mean:   13,532,495.54 stddev:    4,225,220.91 user:    175,360 sys:     34,390 gc:      2,060 mem:     751.39

uptime:  211 acked: 1,503,880 acked/sec:  50,129.33 failed:        0 99%:      36,241,407 99.9%:      46,759,935 min:       6,434,816 max:      72,155,135 mean:   14,540,674.05 stddev:    5,200,081.08 user:    174,870 sys:     35,740 gc:      2,523 mem:     846.15
uptime:  241 acked: 1,503,400 acked/sec:  50,113.33 failed:        0 99%:      41,746,431 99.9%:      68,288,511 min:       6,590,464 max:     137,232,383 mean:   14,784,902.94 stddev:    6,220,659.93 user:    173,800 sys:     36,400 gc:      2,593 mem:     896.85
uptime:  271 acked: 1,504,120 acked/sec:  50,137.33 failed:        0 99%:      35,880,959 99.9%:      49,971,199 min:       6,553,600 max:      77,070,335 mean:   14,223,920.20 stddev:    4,932,482.39 user:    175,290 sys:     35,930 gc:      2,380 mem:     886.29

uptime:  210 acked: 1,616,860 acked/sec:  53,895.33 failed:        0 99%:      32,767,999 99.9%:      48,955,391 min:       6,643,712 max:      73,727,999 mean:   13,798,011.67 stddev:    4,442,751.14 user:    176,310 sys:     34,800 gc:      2,450 mem:     932.71
uptime:  240 acked: 1,503,640 acked/sec:  50,121.33 failed:        0 99%:      34,897,919 99.9%:      53,379,071 min:       6,475,776 max:      75,366,399 mean:   14,001,232.82 stddev:    4,850,826.78 user:    176,380 sys:     34,880 gc:      2,568 mem:     921.85
uptime:  270 acked: 1,503,340 acked/sec:  50,111.33 failed:        0 99%:      33,406,975 99.9%:      45,711,359 min:       6,610,944 max:      65,765,375 mean:   14,058,020.45 stddev:    4,589,290.14 user:    177,140 sys:     35,920 gc:      2,586 mem:     989.75

1.2.0-SNAPSHOT

uptime:  210 acked: 1,503,120 acked/sec:  50,104.00 failed:        0 99%:      36,012,031 99.9%:      54,329,343 min:       6,586,368 max:      86,179,839 mean:   13,866,565.45 stddev:    4,940,470.24 user:    175,790 sys:     32,220 gc:      2,463 mem:     848.33
uptime:  240 acked: 1,503,520 acked/sec:  50,117.33 failed:        0 99%:      32,571,391 99.9%:      48,988,159 min:       6,623,232 max:      74,448,895 mean:   13,639,588.48 stddev:    4,414,394.82 user:    173,890 sys:     32,870 gc:      2,435 mem:     929.12
uptime:  270 acked: 1,503,320 acked/sec:  50,110.67 failed:        0 99%:      30,834,687 99.9%:      43,778,047 min:       6,488,064 max:      64,520,191 mean:   13,545,641.23 stddev:    4,120,231.64 user:    176,190 sys:     32,110 gc:      2,409 mem:     904.40

uptime:  210 acked: 1,503,540 acked/sec:  50,118.00 failed:        0 99%:      37,421,055 99.9%:      61,341,695 min:       6,582,272 max:      85,393,407 mean:   14,000,161.69 stddev:    5,317,874.33 user:    172,530 sys:     33,640 gc:      2,455 mem:   1,001.10
uptime:  241 acked: 1,541,460 acked/sec:  49,724.52 failed:        0 99%:      39,157,759 99.9%:      61,603,839 min:       6,426,624 max:     101,056,511 mean:   14,153,442.96 stddev:    5,630,083.75 user:    173,010 sys:     33,280 gc:      2,617 mem:   1,092.48
uptime:  271 acked: 1,578,560 acked/sec:  52,618.67 failed:        0 99%:      39,780,351 99.9%:      70,909,951 min:       5,787,648 max:     119,734,271 mean:   14,350,305.70 stddev:    5,916,972.37 user:    174,360 sys:     32,960 gc:      2,569 mem:   1,057.77

uptime:  210 acked: 1,541,480 acked/sec:  51,382.67 failed:        0 99%:      32,309,247 99.9%:      46,628,863 min:       6,500,352 max:      79,888,383 mean:   13,589,491.43 stddev:    4,288,111.92 user:    171,970 sys:     33,360 gc:      2,253 mem:     716.99
uptime:  241 acked: 1,540,900 acked/sec:  49,706.45 failed:        0 99%:      34,701,311 99.9%:      52,396,031 min:       6,639,616 max:      92,602,367 mean:   13,937,783.46 stddev:    4,782,018.51 user:    173,120 sys:     32,650 gc:      2,351 mem:     885.23
uptime:  271 acked: 1,503,440 acked/sec:  50,114.67 failed:        0 99%:      31,899,647 99.9%:      44,924,927 min:       6,537,216 max:      69,664,767 mean:   13,662,446.62 stddev:    4,240,397.21 user:    173,980 sys:     33,230 gc:      2,292 mem:     980.86

The difference on CPU usage and latency is reduced again, which looks like closer.
(We need to be aware that test results from same branch fluctuate a bit.)

The new overhead for metrics_v2_replace_meters_to_counters_with_sampling looks like it is in an acceptable range for me.  Right about a 1% difference for latency.  (0.97% to 1.29%) and right about a 1.8% to 1.9% difference for CPU usage.  It is also good to see that even if we turn off the sub sampling there is not too much of an increase in the overhead.
I would be +1 for merging this in (from a performance perspective still need to look at the code in more depth), assuming that we can get similar results for 2.x and with a plan to have the old API deprecated in 2.x and completely removed in a 3.x release.
We also need to make sure that we have the naming convention figured out or some kind of tag support so that we can properly do rollups for different types of metrics.

@HeartSaVioR Thanks for the update. I pulled your changes into the metrics_v2 branch.
@revans2 I'll start working on naming conventions and disallowing certain delimiter characters. If you have any sample paths/names that illustrate your thinking that would be helpful.

@ptgoetz These changes were only for measurement so that's why I made commit titles as WIP. You can just squash two commits into one and free to change author (so that we could squash all the commits into one: I don't like having address review comments in master's commit list).
Btw as someone may notice, I didn't handle multiplying the value accordingly (based on sample rate) so there's bug on the code. I'll address and provide a single commit which addresses all my works.

@HeartSaVioR Agreed/understood. We can squash on merge and cleanup commit messages.
This is a feature branch. You can commit directly if you want.

@ptgoetz @revans2 Addressed the bug (adding sample rate) which is not expected to affect the performance.

+1, @ptgoetz @HeartSaVioR  , are there any more changes planned in this patch? If no can we look at merging asap so that it can be included in 1.2.0?

@arunmahadevan
I think there're something not addressed in this PR, and I don't think we can merge before adding document and having patch for master.
@ptgoetz
Could you revisit this and address remaining comments or file issues if it can be done with follow-up patches? If you don't think they should be addressed, could you leave comments so that we can discuss and make decisions?
@revans2
Could you revisit this and sort out remaining things from your side?

In case of missing, we have unaddressed comments in here:
https://github.com/apache/storm/blob/00a382b017c1e29863ac4d9a4449086ef79384e4/storm-core/src/jvm/org/apache/storm/metrics2/StormMetricRegistry.java#L133-L135
and @revans2 had a voice regarding metadata of metrics, which IMHO looks like non-blocker for the patch, but if it makes sense we need to file a follow-up issue then.

@revans2 (cc @HeartSaVioR ) Regarding metadata, parseable metrics names/paths, etc. what do you think of the following approach?
In a nutshell, make everything configurable (with sane defaults), something along the lines:
METRICS_V2_PREFIX (String prepended to all metrics paths, replacing hard-coded "storm.worker", etc. in current code)
METRICS_V2_PATH_DELIMITER (String/character used to separate metrics path, replaces hard-coded "." in current code.
METRICS_V2_INVALID_NAME_REGEX (Regex that checks user-supplied metrics names for disallowed characters. Would be used to prevent users from inadvertently breaking up a path, for example by putting a "." in a metric name when that's used as the delimiter.
Does that seem reasonable?

@ptgoetz for me this is all about the metadata for the metric.
The code we have been working on for storing metrics in a DB that we can then use for scheduling is getting close to being done.  We have a data structure like the following. (NOTE still a work in progress)
struct WorkerMetric {
  1: required string metricName;
  2: required i64 timestamp;
  3: required double metricValue;
  4: required string componentId;
  5: required string executorId;
  6: required string streamId;
}

struct WorkerMetricList {
  1: list<WorkerMetric> metrics;
}

struct WorkerMetrics {
  1: required string topologyId;
  2: required i32 port;
  3: required string hostname;
  4: required WorkerMetricList metricList;
}

Having metadata separated out for topology, host, port, etc allow us to easily ask questions like how many messages were emitted by all instances of bolt "B" on streamId "default" for the topology "T".  I assume people using ganglia or nagios want to do the same thing without having to setup a separate metric for each bolt/spout/hostname/port combination possible.
Almost every metrics system I have seen supports these kinds of metadata or tags, but almost none of the metrics APIs I have seen support this (dropwizard included).  As such we need a way to parse these values out of the metric name in a reliable way.  Making the name more configurable does not help me in this.  In fact it makes it much more difficult.
The current proposed format is:
String.format("storm.worker.%s.%s.%s.%s-%s", stormId, hostName, componentId, workerPort, name);

It has most of the information we need.  It is missing the stream id and executor id, which for some internal metrics we really need if we are to replace the metrics on the UI and reduce the load on zookeeper.
But the issue for me is how do I get the metadata back out in a reliable way?  topologyid is simple to parse out because there is no '.' character allowed.  hostName is going to have '.' in it, so I don't know what to do there.  ComponentId and streamId I think are totally open so again I am in trouble no matter what I do, and then the name itself we could do some restrictions on to make it simpler.
All I want is some API that can take a metric name and reliably return to me the fields that we put into creating the name.

@revans2 Thanks for clarifying. I have a better understanding of what you're saying now.
How do you feel about just replacing "." with "_" on all metrics path components (host name, componentId, etc.)? That would ensure a path could be reliably split on the "." character.
I'll add stream id and executor id to the path in places where they are available.

@ptgoetz I think that is a good start.  Not sure if users would be upset that their bolt B.1 now looks like B_1.  We might have issues though if someone has both a B.1 bolt and a B_1 bolt.  It might serve them right for doing something so dumb, but could end up being a really painful situation to try and clean up.  I would rather have us look into restricting names for bolts, spouts, and streams.  We could just start out with a warning saying metrics might be messed up if you violate this, and later it becomes an error.

I'm +1 to restrict names to avoid replacing. One thing to consider is that when's what we call later, and personally I think major version is the safest version to do the change like this, so 2.0.0 looks like good milestone to address.
Could we poll how many users are using underscore in one of (component name, stream name, topology name - also hostname?) to user groups, dev groups and Twitter official account, and apply smooth migration to only 1.x version if affected groups are small enough?

@revans2 @HeartSaVioR
I added stream id and executor id to the metrics names, and implemented replacing "." with "_".
One consequence of adding stream id was having to get-or-create metrics on the fly, as opposed to creating them up-front. That means there will be a lookup, string concat, and string replacement on each metrics update, which could affect performance. (Unless I'm missing something obvious, we have to do it that way because we don't know the stream IDs ahead of time and have to instantiate metrics as we see them).
As far as issuing warnings when names contain ".", one option would be to handle the warnings in an ITopologyValidator instance. We could have DefaultTopologyValidator log warnings when certain names contain a ".". We could also provide something along the lines of a StrictTopologyValidator that throws a InvalidTopologyException as opposed to just warning. That might be an easy way to transition from warning to error that also gives users to turn strict checking on or off.
What do you think about that approach?

@ptgoetz
Looks like we compose metric name and lookup from REGISTRY every time, even without executor ID and stream ID. I can see more calculation should be done after addressing, but not sure how much it affects performance. If we could also memorize metric name per combination of parameters it might help, but I'm also not sure how much it will help.
Regarding issuing warning on name, +1 on your approach. Looks nice!

Looks like we compose metric name and lookup from REGISTRY every time, even without executor ID and stream ID. I can see more calculation should be done after addressing, but not sure how much it affects performance. If we could also memorize metric name per combination of parameters it might help, but I'm also not sure how much it will help.

Prior to adding stream ID, we could store the metric as a variable and reuse it without having to do a lookup on each use. Adding stream ID required (unless I'm missing something) doing the lookup on every use. There might be additional optimizations, but because metrics names are composed of several fields, some level of string concatenation is unavoidable. For example, we could try to optimize lookups by caching metrics with metric name as the key, but we would still have to do concatenation to create the lookup key.
I suppose we could create a MetricName class to serve as the cache key, but we'd be trading string concatenation for object creation (unless we name MetricName mutable with setX methods).

Regarding issuing warning on name, +1 on your approach. Looks nice!

Thanks for the feedback. Implemented in latest commit.

@ptgoetz OK got it. Maybe we could reconsider if separation of stream ID is not really needed.
@revans2 Could you take a look at this again?

@ptgoetz , can you rebase ?
@revans2 , can you take a look again so that we can get this in 1.2 ?

@arunmahadevan Rebased.

Hi @revans2 , do you have any additional comments or are we good to merge this ?

I had a couple of comments that weren't addressed. I'm mainly worried about leaking resources in tests/local mode https://github.com/apache/storm/pull/2203#discussion_r155637510, https://github.com/apache/storm/pull/2203#discussion_r155642325.

@srdo Thanks for the review. I think I've addressed your comments in the latest commit.

Was the concern about "My concern here is that most of what we are truly encoding in the name is metadata" addressed?
Is the expectation that the metric name in dashboards is used like storm.topology.{topology ID}.*.*.*.*-myCounter?


Bug: Switching to tab with archive open freezes Files until loaded

Description
Switching to tab with tar file takes a lot of time and UI is completely blocked.
It seems to be even smaller if multiple tar tabs are open.
Steps To Reproduce

Create and open large tar file as per #9848
Switch to previous tab showing normal folder - should be instant
Switch to tar tab - takes about 3-4 seconds meanwhile whole UI is blocked

Expected behavior
Tab switching is instantaneous.
Additionally, UI should be non-blocking.
Files Version
Version: <IP_ADDRESS> OS Version: 10.0.22000.918
Windows Version
Windows 11 21H2 22000.918
Relevant Assets/Logs
#9848

Merging with #9848


Update to<EMAIL_ADDRESS>

Current coverage is 89.47%

Merging #58 into master will not affect coverage as of a13cadb

@@            master     #58   diff @@
======================================
  Files            3       3
  Stmts           19      19
  Branches         0       0
  Methods          0       0
======================================
  Hit             17      17
  Partial          0       0
  Missed           2       2

Review entire Coverage Diff as of a13cadb

Powered by Codecov. Updated on successful CI builds.


Which files need to be hosted?

Hi @wycats @stefanpenner @rwjblue  ,
We are cdnjs team, we're going to host this library.
cdnjs is one of the most famous free and public web front-end CDN services.
What the files should need to be added in cdnjs?
I know that route-recognizer.js and route-recognizer.js.map and<EMAIL_ADDRESS>need to be added.
But what about<EMAIL_ADDRESS>and after v0.2.9's route-recognizer.es.js and route-recognizer.es.js.map ?
Can these files work on web front-end that CDNJS should host them?
If you have any suggestions, please let me know.
Thank you.
cdnjs/cdnjs#11497
cdnjs/cdnjs#8444
cdnjs/cdnjs#8410

I'm doing some issue gardening 🌱🌿 🌷 and came upon this issue. Since it's quite old I just wanted to ask if this is still relevant? If it isn't, maybe we can close this issue?
By closing some old issues we reduce the list of open issues to a more manageable set.
This library is rarely included directly (it's a dependency for other code), so I don't think there is little value in hosting it on a CDN.


Storage Blob Data Reader (Preview) not working

Given Storage Blob Data Reader (Preview) and Reader access to an Azure AD user but when the user is trying to access blobs from the Azure Portal. We are getting below error:
The client<EMAIL_ADDRESS>with object id 'xxxxxxxx' does not have authorization to perform action 'Microsoft.Storage/storageAccounts/listKeys/action' over scope '/subscriptions/subscriptionid/resourceGroups/rbac-storage/providers/Microsoft.Storage/storageAccounts/storageaccount'.
If you would like to access your data using the preview Data Reader or Data Contributor permissions use the quick link in the Overview page.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 90a4bd03-9c86-4c9b-5834-c2966c2f3720
Version Independent ID: a070cc29-15aa-c072-b90b-64c616ef51cc
Content: Authenticate access to Azure blobs and queues using Azure Active Directory (Preview)
Content Source: articles/storage/common/storage-auth-aad.md
Service: storage
GitHub Login: @tamram
Microsoft Alias: tamram

I am able to access Blobs from the Azure Storage Explorer not sure why it's working on Azure Portal.

@vikaskumawat80 Thanks for the feedback! I have assigned the issue to the content author to evaluate and update as appropriate.

Any update on this issue?

@vikaskumawat80 @sghdesign - Yes, we have updated the docs to address this - see here.
To grant portal access to blobs/queues to a user, you need to also assign them the Azure Resource Manager Reader role. That will give them the permissions they need to navigate through the portal to view blob and queue resources.

@vikaskumawat80 - If you are assigning the ARM Reader role and are still not able to grant access, check that you have scoped the Reader role to the storage account or above.

#please-close

I will try that. Thanks.


Missing Staging File

Could you please upload the file name staging.txt that is been used in the demo? I couldn't find it anywhere on zenodo nor here. Thanks!

Sure! I'll put it here in the meantime.
staging.txt

The file is now available in Zenodo
https://zenodo.org/records/13903088


Register-PSResourceRepository help incorrectly shows credential support

Prerequisites

[X] Write a descriptive title.
[X] Make sure you are able to repro it on the latest released version
[X] Search the existing issues.

Steps to reproduce
Step 1: Examine the help returned for Register-PSResourceRepository
PS > get-help Register-PSResourceRepository

NAME
    Register-PSResourceRepository

SYNOPSIS
    {{ Fill in the Synopsis }}

SYNTAX
    Register-PSResourceRepository [-Name] <System.String> [-URL] <System.Uri> [-Credential <System.Management.Automation.PSCredential>] [-Priority <System.Int32>] [-Proxy <System.Uri>] [-ProxyCredential <System.Management.Automation.PSCredential>]
    [-Trusted] [-Confirm] [-WhatIf] [<CommonParameters>]

    Register-PSResourceRepository [-Priority <System.Int32>] [-Proxy <System.Uri>] [-ProxyCredential <System.Management.Automation.PSCredential>] -PSGallery [-Trusted] [-Confirm] [-WhatIf] [<CommonParameters>]

    Register-PSResourceRepository [-Proxy <System.Uri>] [-ProxyCredential <System.Management.Automation.PSCredential>] -Repositories <System.Collections.Generic.List`1[System.Collections.Hashtable]> [-Confirm] [-WhatIf] [<CommonParameters>]

Step 2:
Attempt to pass a credential to Register-PSResourceRepository
 > Register-PSResourceRepository -Name 'testrepo' -URL  $PathToRepo -Trusted -Priority 25 -Credential $cred
Register-PSResourceRepository: A parameter cannot be found that matches parameter name 'Credential'.

Expected behavior
I expect the help to match the cmdlet.

Actual behavior
The help is tragically awry, offering up the barest dream of hope before dashing it against the harsh rocks of reality.

Error details
No response
Environment data
Name| Value
----- | ---
PowerShellGet Version |  `3.0.12`
OS | RedHat 8

**PSVersionTable**

PSVersion                      7.2.4
PSEdition                      Core
GitCommitId                    7.2.4
OS                             Linux 4.18.0-348.23.1.el8_5.x86_64 #1 SMP Tue Apr 12 11:20:32 EDT 2022
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           <IP_ADDRESS>
WSManStackVersion              3.0

Visuals
No response

@randomchance thanks for reaching out about this! Our next release will include a feature to pass Credentials into Register-PSResourceRepository, it's PR #480. We'd love to hear your feedback when this change comes out in our next release, thanks!


Issue with session.solution.report_definitions.volume gives unexpected error

🔍 Before submitting the issue

[X] I have searched among the existing issues
[X] I am using a Python virtual environment

🐞 Description of the bug
When assigning volume reports to session.solution.report_definitions.volume it gives an unexpected error
From fluent GUI i get this error in console:
Error: eval: unbound variable
Error Object: allowed-values
I dont have this issue when assigning  to session.solution.report_definitions.surface
I tried
`session.solution.report_definitions.volume.create('max-afe-chip-temp')
session.solution.report_definitions.volume.create('max-inv-chip-temp')
session.solution.report_definitions.volume.create('max-temp-heatsink-afe')
session.solution.report_definitions.volume.create('max-temp-heatsink-inv')
session.solution.report_definitions['max-afe-chip-temp'] = settings['reportsVolume']['max-afe-chip-temp']
session.solution.report_definitions['max-inv-chip-temp'] = settings['reportsVolume']['max-inv-chip-temp']
session.solution.report_definitions['max-temp-heatsink-afe'] = settings['reportsVolume']['max-temp-heatsink-afe']
session.solution.report_definitions['max-temp-heatsink-inv'] = settings['reportsVolume']['max-temp-heatsink-inv']
`
I get an error for each of the fire last lines, but every time I run the next line the previous line in fluent GUI changes away from invalid. I also tried using method .set_state() but again same error
📝 Steps to reproduce
session.solution.report_definitions.volume.create('max-afe-chip-temp')
session.solution.report_definitions['max-afe-chip-temp'].set_state({'report_type': 'volume-max',
'field': 'temperature',
'average_over': 1,
'per_zone': False,
'zone_names': ['dcb-1.1', 'dcb-1.2', 'dcb-1.3'],
'expr_list': None})
💻 Which operating system are you using?
Windows
🐍 Which Python version are you using?
3.9
📦 Installed packages
ansys-api-fluent==0.3.8
ansys-api-platform-instancemanagement==1.0.0b3
-e git+https://github.com/pyansys/pyfluent.git@472b0c088a0e5040ba23138e87c2cb5184db86c3#egg=ansys_fluent_core
ansys-platform-instancemanagement==1.0.3
appdirs==1.4.4
asttokens==2.2.1
backcall==0.2.0
cloudpickle==2.2.1
colorama==0.4.6
comm==0.1.2
debugpy==1.6.6
decorator==5.1.1
entrypoints==0.4
executing==1.2.0
googleapis-common-protos==1.58.0
grpcio==1.51.1
h5py==3.8.0
importlib-metadata==6.0.0
ipykernel==6.21.0
ipython==8.9.0
jedi==0.18.2
jupyter_client==7.4.9
jupyter_core==5.2.0
matplotlib-inline==0.1.6
nest-asyncio==1.5.6
numpy==1.24.1
packaging==23.0
pandas==1.5.3
parso==0.8.3
pickleshare==0.7.5
platformdirs==2.6.2
prompt-toolkit==3.0.36
protobuf==3.20.3
protoc-gen-swagger==0.1.0
psutil==5.9.4
pure-eval==0.2.2
Pygments==2.14.0
python-dateutil==2.8.2
pytz==2022.7.1
pywin32==305
pyzmq==25.0.0
six==1.16.0
spyder-kernels==2.4.2
stack-data==0.6.2
tornado==6.2
traitlets==5.9.0
wcwidth==0.2.6
zipp==3.12.0

Sean, Mainak any idea what is wrong here?

Could this be related to this issue reported before? https://github.com/pyansys/pyfluent/issues/473. We have faced issues in setting the dictionary directly for volume integral report definition.

Then I have something to look forward  to

@jharming I asked a work experience student to test this with 2023R1. It seems to work just as in 2023R2:

Please let us know if there is some additional nuance missed here. Thanks.

Hi,
first I create the volume report
session.solution.report_definitions.volume.create('max-afe-chip-temp')
Then I assign it
session.solution.report_definitions['max-inv-chip-temp'] = {'report_type': 'volume-max',                                                              'field': 'temperature',                                                              'average_over': 1,                                                              'per_zone': False,                                                              'zone_names': ['inv1d1',                                                               'inv1d2',                                                               'inv1t1',                                                               'inv1t2',                                                               'inv2d3',                                                               'inv2d4',                                                               'inv2t3',                                                               'inv2t4',                                                               'inv3d5',                                                               'inv3d6',                                                               'inv3t5',                                                               'inv3t6'],                                                              'expr_list': None}
When I do this I get an error in script and fluent

If I double click in the fluent report then it changes from invalid to valid

Just to add. If I continue adding the next volume report I get a new error but the "invalid" note from the previous report dissappear.

Just to add. If I continue adding the next volume report I get a new error but the "invalid" note from the previous report dissappear.

Even weirder. I will have another look with the work experience student.

In principle I would just assign all my volume reports like the other reports. i.e. like this
session.solution.report_definitions.surface = settings['reportsSurface']
here I don't create the report first and then assign.
Anyhow first step is to avoid the error in the first place :)

@jharming , iff expr_list is included in the input dict, there is an issue. It is a StringList but has no allowed-values attribute, which is wrong. I will check again the 2023R2 behaviour to see if it's already corrected there. If not, we'll expedite a solution.

@jharming, it's still broken in 2023R2 so we'll work for a fix.
I will try to provide you a fix.

So it will be fixed in 2023r2?

That will be a n immediate fix even for 2023r1. It still shows the error in Fluent but the exception is handled in PyFluent now

@jharming I confirm that set_state calls fail with the Fluent-side allowed-values error iff expr_list is included in the state. As a workaround, you can omit expr_list from the state dictionary, and set_state will behave well (checked with 2023R2 just now and with 2023R1 by our work experience student) . Since you are setting it to None, it is redundant anyway.
My branch solves issues for allowed_values() method calls on parameters like expr_list which omit the allowed-values attribute.
@gyeole @h-krishnan I think we need to resolve the expr_list allowed-values omission in Fluent. Ideally, the base classes can ensure the existence of salient attributes.

@jharming corrected in Fluent 2023 R2


Increase meshery user token expiry time

Signed-off-by: Yash Agarwal<EMAIL_ADDRESS>Description
Earlier the user token was a session cookie (as expiry time was not set). Have set the expiry time to 30 years for now.
This PR fixes #3931

As I said earlier it was a session cookie not persistence.
I tested the code by setting expiry time to 1 sec and 10 sec and it was working perfectly. So I am assuming it would work in this case as well :P

@leecalcote I was trying to increase the time for access token and refresh token as well. But I couldn't find out how to do that.
What I figured out till now is that we receive token as a query param on /api/user/token and after decoding that we get access token, refresh token and their expiry date (which is 1 hr ahead of requested time). But that token is received from the remote provider (as far as I understood) and I have no Idea how to increase that.
I can increase the time like I earlier did to 1 week but although the refresh token function is called after every 1 hr (same as the expiry time of the Provider token currently) (which doesn't update the cache btw) I have no idea how the application would behave.
Also I couldn't find any other solution (have spent 7 hours, still couldn't find it).

Sure @leecalcote


Bazel Build Failed

In running the command: "bazel build ..."
I am seeing following error. Note sure what is causing it.
...........
ERROR: /private/var/tmp/_bazel_USER/3498234242c6f7373b6edebcb02e157b/external/io_bazel_rules_closure/closure/private/defs.bzl:27:16: The set constructor for depsets is deprecated and will be removed. Please use the depset constructor instead. You can temporarily enable the deprecated set constructor by passing the flag --incompatible_disallow_set_constructor=false.
ERROR: error loading package '': Extension file 'closure/private/defs.bzl' has errors.
ERROR: error loading package '': Extension file 'closure/private/defs.bzl' has errors.

I had the same issue. You need to use bazel version 0.5.4.

Should be fixed now, see Dockerfile for updated build instructions.


adopted change in prometheus client lib.

I was not able to build the docker container unless doing the changes contained in this pr. Is it correct? Has the handler been in another package before?

It seems correct, yes. As far as I can tell, prometheus.Handler was deprecated in favor of promhttp.Handler and got finally removed from Prometheus two days ago (see https://github.com/prometheus/client_golang/commit/761a2ff07c763475bff727730e823a8f4d501f36#diff-b83b53c6fd7712ee815387960471957bL53). That's why you are likely the first person to notice this.
Your PR looks good and should fix it, thank you very much. We have plans to improve our dependency management in the future to prevent situations like this.


Rename project

Fix issue #23

maybe just one commit is needed, or?

squash the commits, and when green, you have my 🍏 🚥  😛

I don't think travis is going to do anything anytime soon. I propose to merge the PR as is (gulp runs fine locally).

Apparently, travis passed: https://travis-ci.org/foretagsplatsen/klassified/builds/162703181

I force merge because it seems github is not notified by travis.


Retrieve runtime API metadata

SDK and API operation to retrieve the following:

Resource name
Resource ID (e.g. ARN in AWS)
URL

See proposal at:
https://github.com/nitrictech/nitric/pull/330

Implemented and tested in node-sdk. Once in should be testable with nitric deploy as well (there may be some permissions issues from the CLI that need to be sorted out).


Additional resources section at the bottom of chapters isn't being transferred to assemblies during the split

Most or all chapter files in the source have the following enterprise-only section with additional resources and versioning information (for enterprise legal notice):
ifdef::KOGITO-ENT[]
== Additional resources
* {URL_CREATING_RUNNING}[_{CREATING_RUNNING}_]
* {URL_DEPLOYING_ON_OPENSHIFT}[_{DEPLOYING_ON_OPENSHIFT}_]
* {URL_DECISION_SERVICES}[_{DECISION_SERVICES}_]
* {URL_PROCESS_SERVICES}[_{PROCESS_SERVICES}_]

// Versioning info
include::_artifacts/versioning-information.adoc[]
endif::[]

For example, see the bottom of the configuring chapter file.
Current behavior
This section of additional resources and versioning info at the bottom of the source chapter file isn't transferred to the assembly file during the split.
Desired behavior
The section of additional resources and versioning at the bottom of the source chapter file is transferred to the assembly file during the split. All content from the entire source chapter file should be transferred exactly as is, except for module file content, which of course should be converted to includes.

Should it be added to all chapters?

Well, not that it should be added. I'm saying the writer uses Additional resources whenever needed. So in this case, yes, I've added that section to all chapter files, and is often the norm, but not always. Either way, the splitter isn't adding anything, it was just not carrying that over from the source to the split version for some reason.
In sum, the splitter should copy over everything from the chap source file exactly as is to the new assembly file, except the follow, which it should modify:

Anything that starts with an anchor ID, which is a module, should be converted to includes.
Any includes in the chap file should be converted to includes that correspond to the new split repo (i.e., modified to function given the new location and structure)

Literally everything else from the source chap file besides those two things should just be copied-pasted over to the assembly file during the split. Unless of course is :assembly: false, in which case just take only the module sections and generate the module files from them, skipping the step of doing anything for a new assembly.


Support Request: Hub2 and Eve History with fakegato-history

Describe Your Problem
Eve History for hub
Relevant log output
Is it possible to have data from hub 2 to eve history via fakegato (temperature, humidity, light) ?

Config for homebridge-switchbot
{
            "name": "SwitchBot",
            "credentials": {
                "token": "xxxxx",
                "secret": "xxxx",
                "notice": "Keep your Token & Secret a secret!"
            },
            "options": {
                "devices": [
                    {
                        "deviceId": "XXXXXXXXXXXXXXX",
                        "configDeviceType": "Hub 2",
                        "refreshRate": 30,
                        "offline": false,
                        "history": true,
                        "external": false
                    }
                ],
                "irdevices": [
                    {
                        "deviceId": "XXXXXXXXXXXXXXXXXXXXXXX",
                        "configDeviceName": "Télécommande Sony grande",
                        "hide_device": true,
                        "configRemoteType": "TV",
                        "connectionType": "OpenAPI",
                        "customize": false,
                        "disablePushOn": true,
                        "disablePushOff": true,
                        "disablePushDetail": false,
                        "external": false
                    }
                ],
                "refreshRate": 30
            },
            "platform": "SwitchBot"
        },

Screenshots
No response
Device & Model
Hub 2
Node.js Version
v16.20.2
NPM Version
v8.19.4
Homebridge Version
v16.20.2
Homebridge SwitchBot Plugin Version
v2.8.2
Homebridge Config UI X Plugin Version
v4.50.5
Operating System
Raspbian

Should be able too. I will look into this.

Can you try the latest beta:

Click here to find out how to install Beta

It works for humidity and temperature with beta version
Not for the lightning level
Thanks for the work

Sorry totally forgot, I will get a new beta out in a little bit.

why sorry ? ;-) things happened ...


LlamaIndex integration requirement

as title...

+1 to this.

I would be interested to see a snippet of code showcasing your use case and what you would expect from the Chainlit integration.
It would help to make the requirements of the integration explicit :)

Hi. I am using LlamaIndex and for the past 2 days it has been giving errors in the output string. It repeats the content of the previous output in the response. Have the latest LlamaIndex enhancements affected the outputs? Please help/ advise. thanks

+1

We are starting to think about this integration. If you guys could provide more context about your use cases that would help us a lot!

Hello guys, we are starting to work on this. Draft PR here. Would love some feedback!

The latest release is introducing a first version of our Llama Index integration!


Fix try/catch/finally example

It actually compiles now and also includes how to log an exception

Coverage remained the same at 73.368% when pulling ced1d0c34aa298aa02711a706f2f87f6b8732460 on felixbarny:patch-2 into c187d9db735943575acdf9333acf84bb45d93849 on opentracing:master.

thanks!


[Port 2.0.0-rc.5.0] Hide part of code behind feature flag (#22529)

Original: https://github.com/microsoft/FluidFramework/pull/22529
Part of a hotfix
There is some minimum sequence number shenanigans going on. Not exactly sure, but we want to be able to safely turn off the feature.

Approved patch


Install script INSTALL_K3S_PR support does not work if CI has run more than once

If the action runs multiple times (say due to being restarted to address a flake), the RUN_ID expression may return multiple results:
https://github.com/k3s-io/k3s/blob/1d22b6971fbf268587335d3b13f359609f76c7cc/install.sh#L496-L497
#8 1.395 + RUN_ID='9118227575
#8 1.395<PHONE_NUMBER>'
#8 1.396 ++ curl -s -H 'Authorization: Bearer xxx' 'https://api.github.com/repos/k3s-io/k3s/actions/runs/9118227575
#8 1.396<PHONE_NUMBER>/artifacts'
#8 1.402 + artifacts=

cc @dereknola

no backports, since install script changes only go to master

Validated using PR number 10113 on master branch
Environment Details
Infrastructure
Cloud EC2 instance
Cluster Configuration:
Single node
Steps to validate

export GITHUB_TOKEN
Install install a merged commit where CI ran more than once

> curl -fL https://get.k3s.io | INSTALL_K3S_PR=10113 sh -s -
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 35365  100 35365    0     0   124k      0 --:--:-- --:--:-- --:--:--  125k
[INFO]  Using PR 10113 as release
[INFO]  Downloading hash https://api.github.com/repos/k3s-io/k3s/actions/artifacts/1511389615/zip
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  120M  100  120M    0     0  26.2M      0  0:00:04  0:00:04 --:--:-- 28.3M
[INFO]  Verifying binary download
[INFO]  Installing k3s to /usr/local/bin/k3s
[INFO]  Skipping installation of SELinux RPM
[INFO]  Creating /usr/local/bin/kubectl symlink to k3s
[INFO]  Creating /usr/local/bin/crictl symlink to k3s
[INFO]  Creating /usr/local/bin/ctr symlink to k3s
[INFO]  Creating killall script /usr/local/bin/k3s-killall.sh
[INFO]  Creating uninstall script /usr/local/bin/k3s-uninstall.sh
[INFO]  env: Creating environment file /etc/systemd/system/k3s.service.env
[INFO]  systemd: Creating service file /etc/systemd/system/k3s.service
[INFO]  systemd: Enabling k3s unit
Created symlink /etc/systemd/system/multi-user.target.wants/k3s.service → /etc/systemd/system/k3s.service.
[INFO]  systemd: Starting k3s


Please add a confirmation to closing the app

Type: Feature Request
It is such a basic feature and such a pain in the butt on miss click that the whole dam thing closes without confirmation. please
VS Code version: Code 1.87.2 (Universal) (863d2581ecda6849923a2118d93a088b0745d9d6, 2024-03-08T15:21:31.043Z)
OS version: Darwin arm64 23.1.0
Modes:

Did you try the window.confirmBeforeClose setting?

@gjsjohnmurray suggestion should be what you're looking for.

amazing thank you 🙏


[Feature request]*2: force-download; other-engines

Two questions/feature requests: anything I can do to help?
(1) I'm forced to use a VPN to access google.  So, I wonder if it's possible for me to easily adapt this project to other search engines?  Have you done any significant work to make Bing possible or do you have any plans to?
(2) Maybe for the same reason or perhaps other reasons, google_image_search sometimes doesn't return any image results For example, I ask for 5 images, and it says it can't download any of the images; if I try again later, it downloads all five--I don't know why I get this intermittent error.  Maybe it's because VPNs are not stable?  But I feel that's unlikely: my VPN has been very stable, and I get the same issue on ShadowSocks.
Can we implement a feature that will force a minimum quantity of downloads to succeed for each term?  Cause the script to continue working until it gets five unique pictures unless the user interrupts it?  Right now, only maybe 20% of the time 5/5 images are collected.
Is there some way I can help to debug this?  I'm a very novice programmer, but I can follow directions well, and I can make time.

Hello, I do understand your issue, that we could easily just integrate other search engines. I have already successfully made a downloader for duckduckgo.com, I'll see later if I could integrate it with this library


ValidateSnippetCompletionAfterPlaceholderReplacements - ensure 'combined' contains the original file + snippet insertion

When reviewing #2942, I noticed the 'combined' files just contain the snippet with placeholders, and not the result of accepting the snippet into the original file.
Retaining the original file context is necessary to unblock cases where the the file contains necessary pre-requisites (e.g. setting targetScope).

@anthony-c-martin, thanks for looking into this.


Adding the solution pattern for Change Data Capture(CDC)

This is an architectural pattern for CDC which is based on my article https://medium.com/event-driven-utopia/a-gentle-introduction-to-event-driven-change-data-capture-683297625f9b

Fix #27


 I can't watch the video w/out signing into Slack.

Sign into slack. Github is limited.
GJ

On Aug 12, 2018, at 10:36 PM, Ryan Nystrom<EMAIL_ADDRESS>wrote:
Can you upload a gif? I can't watch the video w/out signing into Slack.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/GitHawkApp/GitHawk/issues/2109#issuecomment-412394251, or mute the thread https://github.com/notifications/unsubscribe-auth/AJqdYkw7WkEgDN0yh-OX7Z-uxrpKwdx_ks5uQOY7gaJpZM4V5zYf.

I don't use Slack 🤷♀

Hi Ryan,
I am confused. I thought I was responding to a dev member.  I am using GitHawk to communicate with my team. Wondering how you got access to our private project? I know githawk needs acces but not sure why you have access?
Are you an iOS and android developer?
Do you happen to know react?
I am looking for good developers.
GJ
Sent from my iPhone

On Aug 13, 2018, at 3:14 PM, Ryan Nystrom<EMAIL_ADDRESS>wrote:
I don't use Slack 🤷♀
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.


Composer Broken in Yii2

I have the latest version of Yii2 Advanced installed.
I have tried to install JUI and Bootstrap using composer and the install fails each time.
The errors are always as follows:
Your requirements could not be resolved to an installable set of packages.
Problem 1
- yiisoft/yii2 2.0.3 requires bower-asset/jquery 2.1.@stable | 1.11.@stabl
e -> no matching package found.
- yiisoft/yii2 2.0.3 requires bower-asset/jquery 2.1.@stable | 1.11.@stabl
e -> no matching package found.
- Installation request for yiisoft/yii2 == <IP_ADDRESS> -> satisfiable by yiisoft
/yii2[2.0.3].
Potential causes:

A typo in the package name
The package is not available in a stable-enough version according to your min
imum-stability setting
see https://groups.google.com/d/topic/composer-dev/_g3ASeIFlrc/discussion f
or more details.

Read http://getcomposer.org/doc/articles/troubleshooting.md for further common
problems.
Installation failed, reverting ./composer.json to its original content.

http://www.yiiframework.com/doc-2.0/guide-start-installation.html:
composer global require "fxp/composer-asset-plugin:1.0.0"


Change to unpublished post title doesn't update post url

When I change the title of an unpublished post, the post URL doesn't update. I understand that if a post has already been published, changing the URL might not be desired, but prior to publication I'm often making dramatic changes to the post title, which I'd like reflected in the URL. What makes this more pressing is that the post URL isn't displayed on the same screen, so it's very easy to miss.

I can identify with this. Currently I have the strangest slugs but I'm in the habit of checking now. We'd need an elegant way to achieve this.
We could probably trigger with auto-save or with manual save.
We'd need to consider the various things to check:

did the user edit the post title
did the user manually override the slug in the past - it might be irritating if you've already got a slug sorted out but you make a refinement to the post title and your slug gets changed.
we'd need to check if post is published or not

It might however be best to submit this on the ideas site: http://ideas.ghost.org/forums/285309-wishlist

I can definitely see how this feature would make that easier, as it is something I have thought about before, but I personally think that it should not be changed. My reason is that it could be a problem is if someone writes a title, saves a draft, checks the slug, likes it, and then decides to make one quick title change before publish, now what they checked is no longer valid. I think it is a lot easier to update the slug when I make an update to the title if I want the url to change, then be constantly checking it to make sure its correct, or didn't change so I don't publish an article with a bad url.
Url's are very important to many people, and I think beyond just creating one when the post is initially saved, Ghost shouldn't be touching that based on what it thinks the user wants.

@dbalders
You could do what @nuclearpengy suggested and check whether the user has already changed the slug. If they have, leave it alone.

Checking if the user has changed the slug is nontrivial. We'd need to make another column to keep track of something like that, and I don't really see something like that happening. So it'll come down to a design decision from the people up top.
The reason I don't see this happening is because we have no way of knowing if the user has changed the slug. If someone set the title, then customized the slug, then changed the title again, they probably don't want the slug to change.
I'm going to close this for now; please feel free to create a feature request at our user wishlist though; if we see this start to get traction we'll definitely revisit it! Thanks for taking the time to raise the issue, @jonblack

@novaugust
By another column I assume you mean in a database. Is that really needed? When the slug is generated, it's done so in a predicable manner (a post title will always produce the same slug), so if it doesn't match what's expected, the user has changed it. There are probably edge cases I'm not thinking of, but I thought it useful to add this comment in case someone comes looking.

fwiw I agree with this issue and @jonblack's reasoning above sounds pretty sane to me. This problem is really an unintended consequence of introducing auto-save - I've bumped into it several times.

Not that this is how we should do it, but the workflow works pretty well, I'd reckon this would be regarded as a pretty standard 'core' feature.

click edit

I have no idea what's technically happening behind the scenes, but it works pretty well.
</chirping from the sidelines>

Whilst not strictly a bug, because we built it this way on purpose, this behaviour does need changing now that we have autosave.
I think it should be possible to keep track of whether the current slug is generated from the current title, and then on a title change, if the slug is generated update it. If it is a custom slug, leave it alone. Definitely quite fiddly though, and we need to ensure we don't auto-update the slug after the post is published.

Why don't we add another status... let me explain (although I am not sure of the full post lifecycle):

Draft: the post is a draft, never been published.
Published: the post is now public.
Unpublished: the post has been pulled back from published into draft again.

This will enable us for having better traceability in the future (as the user will be able to quickly track the posts that the public has already viewed).
Then the solution to this "bug" (I think it is a feature change) will be as trivial as change the slug when the post is on the "Draft" status and don't change it (just do it manually) when the post is in any other status.

There's no need to add an extra status to determine if a posts has been published and then unpublished, as the post will have a status of draft but have a published_at date set, although that will change in future.
However I think the word 'unpublished' is confusing in the original issue title/description, as it's actually (as I understand it) referring to any draft post, regardless of whether it has or hasn't been published.

@ErisDS your assumption is correct, I was indeed referring to a draft post

I am of the opinion that a database field is overkill here. Whilst it's true that the only way to know 100% if the field has been edited by the user is to track edits and then set a boolean, even then it's still impossible for us to detect user intention:
E.g in the case that the user edits the field, but the result is no change, is the intention that they meant to not edit the field, or that they meant for the field to stay as it is regardless of future changes to the title?
By treating the slug as a sort of hashed form of the title, we can verify if any changes have been made to the slug. So when there is a change to the title we know both what the title was, and what it is now. Using what the title was we can verify that the slug is a valid slugified version of the old title. If that's true and the post is a draft the slug is safe to update, otherwise don't update the slug.
This should result in a significant improvement to the behaviour of the slug without having to introduce the complexities of storing a flag.

Very elegant solution. I'll work on it today and I'll update the PR.

The only problem I can see with it is when the user has published the post, for some reason, goes back to draft and then the title is changed. When the post is published again the slug will be from the new title but the user might expect to have the old slug as it was published before. Are we ok with this behaviour?

I think the comment above by @ErisDS answers your question:

If that's true and the post is a draft the slug is safe to update, otherwise don't update the slug.

Slug is not updating when the post is draft anymore... is this a desired effect or is it another bug?
e.g. Create a new draft, save it, edit the title, save it and the slug didn't change.

@dgonzalez I'm a bit confused by your comment - what you're describing is the exact bug that is raised here?


Text of all notes needs to be overworked. Show odds, multiplier, and some nice text and emojis

I guess it's good for now :)


feat(meta): watch and reload license key from file

Signed-off-by: Bugen Zhao<EMAIL_ADDRESS>hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Add a CLI argument of --license-key-file for the meta node, which enables a background task to watch and reload license key from that file, simplifying license key rotation in Cloud.
Some notes:

Only take effect on the meta node, as the license key will still be propagated through system parameters to all other nodes.
Can also be configured with env var RW_LICENSE_KEY_FILE.
Manually specifying the license key via system parameters (license_key), initial configuration (system.license_key), or the RW_LICENSE_KEY environment variable is still possible but not recommended when using --license-key-file, as it can easily get overwritten if the key file's content changes.

Added a unit test.
Checklist

[x] I have written necessary rustdoc comments
[x] I have added necessary unit tests and integration tests
[x] All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

[ ] My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note
See description above.

Manually specifying the license key via system parameters (license_key), initial configuration (system.license_key), or the RW_LICENSE_KEY environment variable is still possible but not recommended when using --license-key-file, as it can easily get overwritten if the key file's content changes.

How about adding an error message for ALTER SYSTEM SET license_key TO '...' to notify users they are using it incorrectly.

How about adding an error message for ALTER SYSTEM SET license_key TO '...' to notify users they are using it incorrectly?

Oops. Missed this. Will refine in the following PR.


[5.2] Break recursive exception handler

Breaking the recursive execution of exception handler when got error in the midle of handler.
Related issue #15043

What if you want to handle more than one exception?

What if you want to handle more than one exception?

Is it posible?
If want to handle squence exception (not nested), it may can add self::$exceptionLevel--; in the last of method.

Holding off on this implementation.


Code Design for intermediate processing steps requiring external or dependent parameters

I was searching for sample use cases for applying masks in echoflow. Specifically, we're utilizing the frequency_differencing function and employing it as a mask. The challenge arises from our utilization of lists for freqAB and chanAB, and echoflow using YAML configuration. The question is: how should I accommodate the input, considering that these lists are generated from the "sv" output?
Here's an example of how we're currently approaching this:
freqAB = list(out_ds.frequency_nominal.values[:2])         freqdiff_da = ep.mask.frequency_differencing(source_Sv=out_ds, freqAB=freqAB, operator=">", diff=5)
This is merely a singular instance; there could exist additional functions that also rely on preceding data. An illustration of this is the add_location function, which requires both an xarray dataset and an echodata object to perform a location update.
@leewujung @lsetiawan @valentina-s

Passing a file after existence check wherever a file is passed.
Example:
- name: echoflow_add_location
    module: echoflow.stages.subflows.add_location
    options:
      use_offline: true
    external_params:
      echodata: 'path/to/file'

Closed in #45