Feature: [vmagent] added push data with zabbix connector #6087
Conversation
…magent Signed-off-by: Sinotov Vladimir <vladimir@sinot.xyz>
SinotQ
changed the title
There was a problem hiding this comment.
Hello @SinotQ, thank you for the PR!
Please, see a few comments/questions.
Disclaimer: I'm not very familiar with Zabbix, so there are mostly questions about logic in parser which might be specific to Zabbix.
Also, it would be great if you could update documentation to include an instruction on how to use this integration with Zabbix. This is not mandatory, but will be much appreciated.
See this part of documentation for an example - https://docs.victoriametrics.com/single-server-victoriametrics/#how-to-send-data-from-newrelic-agent
| zabbixconnectorHistoryErrors.Inc() | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| fmt.Fprintf(w, `{"error":%s}`, strconv.Quote(err.Error())) |
There was a problem hiding this comment.
| fmt.Fprintf(w, `{"error":%s}`, strconv.Quote(err.Error())) | |
| fmt.Fprintf(w, `{"error":%q}`, err.Error()) |
| zabbixconnectorHistoryErrors.Inc() | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| fmt.Fprintf(w, `{"error":%s}`, strconv.Quote(err.Error())) |
There was a problem hiding this comment.
| fmt.Fprintf(w, `{"error":%s}`, strconv.Quote(err.Error())) | |
| fmt.Fprintf(w, `{"error":%q}`, err.Error()) |
| // See https://www.zabbix.com/documentation/current/en/manual/appendix/protocols/real_time_export#item-values | ||
| r.Timestamp = cl*1e3 + ns/1e6 | ||
|
|
||
| if (par & ADDGROUPS) != 0 { |
There was a problem hiding this comment.
Could you move addGroups and addEmptyTags command-line flags definition to this package and use their values directly instead of passing par value with bitmask comparison?
It should make it easier to read and understand the code here.
| const ( | ||
| ADDGROUPS byte = 0b00000001 | ||
| ADDEMPTYTAGS byte = 0b00000010 | ||
| EMPTYVALUE byte = 49 // "1" |
There was a problem hiding this comment.
Could you elaborate on why 1 is used as empty value? Is this something specific for zabbix?
If not, wdyt about using something like [empty] and making this configurable by user?
Also, constants in Go are named in CamelCase where the first letter will make constant exported or private. As far as I can see, emptyValue can be used here since constant is only used by this package so it can be unexported.
| tag = r.addTag() | ||
| tag.Key = append(tag.Key[:0], k...) | ||
| if len(v) == 0 { | ||
| tag.Value = append(tag.Value[:0], EMPTYVALUE) |
There was a problem hiding this comment.
Could you share a use-case when empty labels must be kept?
| return fmt.Errorf("missing `host` element in `host` object") | ||
| } | ||
| tag := r.addTag() | ||
| tag.Key = append(tag.Key[:0], []byte("host")...) |
There was a problem hiding this comment.
Usually, in Prometheus like systems this label should be instance.
See this for example.
| tag.Key = append(tag.Key[:0], []byte("host")...) | |
| tag.Key = append(tag.Key[:0], []byte("instance")...) |
Also, for Prometheus-like systems there should be a label named job but I don't see anything similar in Zabbix. Am I missing something that can be used in similar manner or there is no similar label?
| } | ||
|
|
||
| skipEmptyTags := (par & ADDEMPTYTAGS) == 0 | ||
| item_tags, err := getArray(o, "item_tags") |
There was a problem hiding this comment.
| item_tags, err := getArray(o, "item_tags") | |
| itemTags, err := getArray(o, "item_tags") |
Please, check out make check-all for code-style suggestions.
Description
Support receiving data from the Zabbix connector. (vmagent)
Added
/zabbixconnector/api/v1/history-zabbixconnector.maxLineLen(similar to the-import.maxLineLenflag)-zabbixconnector.addGroups(Enable adding Zabbix host groups to labels (group_name="1"))-zabbixconnector.addEmptyTags(Enable adding Zabbix tags without values to labels (tag_name="1"))Note
__name__label.hostlabel.hostnamelabel.Otherwise, I tried to adhere to the functionality of
/api/v1/importUseful links: