Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add node groups to opcua input plugin #8389

Merged
merged 21 commits into from
Dec 3, 2020
Merged

Add node groups to opcua input plugin #8389

merged 21 commits into from
Dec 3, 2020

Conversation

reimda
Copy link
Contributor

@reimda reimda commented Nov 11, 2020

Remove unneeded node settings (data_type and description).

Add the concept of a node group. Groups have default settings for all nodes in the group. They also are able to override the plugin's metric name so multiple metric names can be used in a single plugin instance.

Remove the name tag which wasn't useful because it duplicated the OPC UA value's field name.

Expose stats to telegraf internal plugin.

Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

## identifier - tag as shown in opcua browser
## data_type - boolean, byte, short, int, uint, uint16, int16,
## uint32, int32, float, double, string, datetime, number
## field_name - field name to use in the output
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this documentation is a bit confusing; I thought these were top-level settings until I got to the example below. Not sure what we can do to improve it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nodes was a top level setting before this PR. I added groups in this PR and each group also has a set of nodes. I left in the top level nodes for backward compatibility. I can see that this is confusing so I'll have to work on the docs.

plugins/inputs/opcua/opcua_client.go Outdated Show resolved Hide resolved
plugins/inputs/opcua/opcua_client.go Outdated Show resolved Hide resolved
plugins/inputs/opcua/opcua_client.go Show resolved Hide resolved
@reimda
Copy link
Contributor Author

reimda commented Nov 17, 2020

I reverted the changes that affect backward compatibility. I think you both are right that this plugin has been released long enough that we should keep compatibility.

I also added a feature OPC UA users have requested, which is to be able to add tags to specific node IDs or to groups. Sorry for sneaking this feature into this PR after the initial review but it seemed closely related and not big enough to start a separate PR.

@srebhan srebhan self-assigned this Dec 2, 2020
for i, item := range o.NodeList {
nameEncountered := map[metricParts]struct{}{}
for i := range o.nodes {
node := &o.nodes[i]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you get this directly in the for loop above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, thanks

Comment on lines +321 to +342
func newMP(n *Node) metricParts {
var keys []string
for key := range n.metricTags {
keys = append(keys, key)
}
sort.Strings(keys)
var sb strings.Builder
for i, key := range keys {
if i != 0 {
sb.WriteString(", ")
}
sb.WriteString(key)
sb.WriteString("=")
sb.WriteString(n.metricTags[key])
}
x := metricParts{
metricName: n.metricName,
fieldName: n.tag.FieldName,
tags: sb.String(),
}
return x
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function might benefit speedwise from the optimization done here: #8391.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this validation is only needed once during plugin initialization I didn't think optimization was worthwhile

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two very minor comments. Looks good to me.

@reimda reimda changed the title OPC UA input plugin: remove useless settings and add node groups Add node groups to opcua input plugin Dec 3, 2020
@reimda reimda merged commit 498a6da into master Dec 3, 2020
@reimda reimda deleted the opcua-fix branch December 3, 2020 00:06
@reimda reimda linked an issue Dec 4, 2020 that may be closed by this pull request
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

inputs.opcua: schema improvements and fix static tag additions
4 participants