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

Already on GitHub? Sign in to your account

[WIP] Support a virtical line #56

Merged
merged 7 commits into from Mar 5, 2014

Conversation

Projects
None yet
3 participants
Collaborator

hirose31 commented Mar 3, 2014

@kazeburo
まだ最低限ですけどざっくり縦線実装してみましたんですがこんなんでどうですかね?というプルリです。

  • エンドポイント
    • /vrule/:service_name/:section_name/:graph_name
      • 特定グラフのみに縦線を描く
    • /vrule/:service_name/:section_name
      • 特定セクション配下の全グラフに縦線を描く
    • /vrule/:service_name
      • 特定サービス配下の全グラフに縦線を描く
    • /vrule
      • 全グラフに縦線を描く
  • パラメータ
    • time
      • 縦線を描く時刻。任意。デフォルトは今。過去の時刻も指定可能。
    • color
      • 縦線の色。任意。デフォルトは #FF0000
    • description
      • グラフに表示する判例文字列。任意。

curl -X POST -F 'color=#00FF00' -F 'description=test!' http://127.0.0.1:5125/vrule/test/hirose/foo

実装メモ

  • エンドポイントの頭を /vrule/ にした理由
    • 既存の /api/ だと、パラメータ(例えば mode=vrule)によって既存コード中に違う挙動のコードを入れないといけないんでちょっときもいので却下
    • 既存の /graph/ は、グラフの見た目調整用なわけで、縦線は見た目よりかデータ登録だと思うので違うかなーと思い却下
  • GF::RRD#graph でdbhを使うのに、GF::Webから$self->rrd->graphを呼ぶときに $self->data を渡すようにしてるのがきもいかも? GF::RRDの中で GF::Web::data 相当のことができればいいのかもすけど。。

TODO

  • 起動時に--enable-vruleが指定されたときのみ有効にすべきか? →なくていい
    • GF::RRD#graphでDB参照してるので、縦線不要な人も性能劣化するのが心配
  • 他のテーブルと同じようにidカラムを追加する。API操作のために。 13541ec
  • 凡例のところ、縦線が増えると下に伸びて行きそうなので、消せるオプション欲しい(デフォルトは表示で) ad02485
  • GET /vrule/.. でJSONが取れるように 47cff57
  • sqliteで動作確認する
  • documentation
Owner

kazeburo commented Mar 4, 2014

縦線が表示されてるスクリーンショットをみたいです

Collaborator

hirose31 commented Mar 4, 2014

この表示期間だと4本の縦線が登録されてて、内1本は/test/hirose/foo固有指定で作った縦線なので、上のグラフには描かれてるけど、下のグラフにはない感じのスクリーンショットです。

vrule

Collaborator

hirose31 commented Mar 4, 2014

idカラムないとAPIで操作するとき辛いですね。

Owner

kazeburo commented Mar 4, 2014

スクリーンショットありがとうございます!
凡例のところ、縦線が増えると下に伸びて行きそうなので、消せるオプションがあるといいですね
デフォルトは表示でいいかな

あと、GET /vrule/.. でJSONが取れればよさげ

Collaborator

hirose31 commented Mar 4, 2014

@kazeburo
実装系はこんな感じかなと思うんで、いったん見てもらえると助かりますー

@kazeburo kazeburo added a commit that referenced this pull request Mar 5, 2014

@kazeburo kazeburo Merge pull request #56 from hirose31/feature/virtical-line
[WIP] Support a virtical line
348e4b9

@kazeburo kazeburo merged commit 348e4b9 into kazeburo:master Mar 5, 2014

@sonots sonots commented on the diff Mar 6, 2014

lib/GrowthForecast/Data.pm
@@ -108,6 +108,20 @@ CREATE TABLE IF NOT EXISTS complex_graphs (
UNIQUE (service_name, section_name, graph_name)
)
EOF
+
+ $dbh->do(<<EOF);
+CREATE TABLE IF NOT EXISTS vrules (
+ id INTEGER NOT NULL PRIMARY KEY,
+ graph_path VARCHAR(255) NOT NULL,
@sonots

sonots Mar 6, 2014

Collaborator

すでに閉じてる pull req にコメントしてしまってすみませんが、
service_name, section_name, graph_name がそれぞれ varchar(255) なので、
graph_path が varchar(255) だと文字サイズが足りなくてエラーが出る可能性があります。

そんなに長いパスを使う人はないとは思いますが、整合性は取っておいたほうがいいのではないかな、と。

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