fluent / fluent-plugin-mongo

MongoDB input and output plugin for Fluentd
https://docs.fluentd.org/output/mongo
173 stars 61 forks source link

Tag collection mapping #2

Closed hotchpotch closed 13 years ago

hotchpotch commented 13 years ago

日本語で失礼します。

タグごとに別のコレクションに保存したかったので、tag_collection_mapping という機能を実装しました。foo.bar.baz というタグならコレクション名 foo.bar.baz に自動で保存されるようになります。

また、設定の remove_prefix_collection でコレクション名から不必要な接頭字を削除するオプションも追加しました。これは remote.routing. のような転送用の prefix のタグを削除してコレクション名にしたかったためです。

ソースコードがそこそこ変わってますが、良かったら取り込んでもらえますでしょうか?なおパフォーマンス劣化はほとんど無いとは思っていますl。

repeatedly commented 13 years ago

Thanks to pull request!

有用な機能だとは思うのですが,これだけ挙動に違いがあるのであれば, 継承などして他のクラスにしてしまった方が今後のメンテナンスも楽な気がしてますがどうでしょう?

frsyuki commented 13 years ago

バッファの"key"を使うと、コードを減らしてメンテナンス性を向上できそうです。

def emit(tag, es, chain)
  if @tag_collection_mapping
    collection = … # コレクション名の整形
  else
    collection = @collection
  end
  super(tag, es, chain, collection)
end

このようにemitをオーバーライドしてやると、def write(chunk) の中でコレクション名を取得できます:

collection = chunk.key

性能的にも優れています。あとは operate(records) を operate(chunk.key, record) に変更すれば対応可能です。

ただ注意点として、BufferedOutputTestDriverがkeyの指定を想定した実装になっていないので、テストが書きにくくなる問題はあります(テストドライバ側に改善が必要)

この機能、あると嬉しい人は多いと思います。他のNoSQL系プラグインのリファレンス実装としても良い感じです。

hotchpotch commented 13 years ago

repeatedly さん

複数 plugin の名前空間ができるのはどうなんだろう、と思って別けましたが、複数の名前空間でも方が良い気もしてきました。あとでクラス別けてまたコメントしますね。

frsyuki さん アドバイスありがとうございます、その方法に変更してみますね。BufferedOutputTestDriver のほうも後方互換性に影響が無く改善できたら fluentd のほうも変更しようと思います。

hotchpotch commented 13 years ago

frsyuki さん

emit を上書きする方法で write の chunk.key を使った方法だと、chunk.key に一つのコレクション名しか記録できなくなりませんか?

   def format(tag, time, record)
     [tag, record].to_msgpack
   end                    

   def write(chunk)
    collections = {}
    chunk.msgpack_each { |tag, record|
      record[@time_key] = Time.at(record[@time_key]) if @include_time_key
      (collections[tag] ||= []) << record # ココ
    }

の実装なので、chunk#msgpack_each の内部でどのコレクションに振り分けるかの実装になってるので、chunk.key で一つしかとれないとまずいのです。

frsyuki commented 13 years ago

提案の方法ではemitの段階でどのコレクション名を振り分けているので、chunk#msgpack_each の内部で振り分ける必要はありません。BufferedOutputが同じkey(コレクション名)のレコードは同じchunkに追記していってくれます。

frsyuki commented 13 years ago

>お二方 こういう実装を想定していたのですが、ぶっちゃけコレでどうでしょう? 40d6f31f455ef1b82faf83dcece517629add8732

hotchpotch commented 13 years ago

なるほどー、実際の実装のサンプルありがとうございます。

ただクラス分けした方が良いか?という疑問と emit で毎回 proc を評価してるので遅くならないかな、という疑問があります。また個人的には proc 使っての呼び出しの切り分けはあまり綺麗では無いと思っています。

クラス分けした実装のほうはこのブランチにすでに push してありますので、クラス分けの実装の方が良さそうなら emit での低コストの実装を取り入れもうちょい手を加えようと思いますがどうでしょうか。

全く関係ないですが、github の pull request、コミットツリーを rebase してしまうと Web UI からの自動マージができないんですね!

repeatedly commented 13 years ago

@frsyuki

実装のサンプルとしてはいいと思います.ただTwitterでも述べた通り結構挙動が変わるのと,それがconfigの一パラメータで切り替わるのはあまり直感的ではないかと個人的には思っているので,クラスを分けたい所です.

@hotchpotch

@frsyuki へのreplyでも述べてますが,個人的にはクラスを分けた方が全体としてはすっきりすると思っているので,問題ないのであれば,クラス分けた方で進めて頂けると有り難いです!

frsyuki commented 13 years ago

@repeatedly, @hotchpotch

クラス分けるのはどっちでもアリだと思います。 Twitterでもreplyしましたが、emitの低コスト化は非常に良さそうです。期待!

hotchpotch commented 13 years ago

だいぶ遅くなりましたが、emit にまとめて低コスト化しました。 取り込んでいただけると嬉しいです :D

repeatedly commented 13 years ago

I merged this pull request and released new version!

Thanks!