Opcua input plugin + authentication options #8009

chrishayles · 2020-08-20T00:28:11Z

Required for all PRs:

Signed CLA.
Associated README.md updated.
Has appropriate unit tests.

Opcua plugin. Adapted from PR: 7213
#7213

Changes include authentication and encryption options.

ssoroka

Thanks for the plugin. Have some change requests for you.

plugins/inputs/opcua/opcua_client.go

ssoroka · 2020-08-20T19:04:29Z

plugins/inputs/opcua/opcua_client.go

+	ReadSuccess  int
+	ReadError    int
+	NumberOfTags int


These would be good as selfstat.Stat instances. See other plugins for examples. selfstats are reported automatically as part of [[inputs.internal]]

ssoroka · 2020-08-20T19:07:20Z

plugins/inputs/opcua/opcua_client.go

+	Interval       string   `toml:"time_interval"`
+	TimeOut        int      `toml:"timeout"`


should both be a config.Duration?

There may have been a solid reason for interval using a string. I'll look into it and change as appropriate.

Did you look into this? It seems error prone to allow people to set the interval as a string. The docs say 5s or 10s which is the format config.Duration uses.

plugins/inputs/opcua/opcua_client.go

plugins/inputs/opcua/README.md

metric/builder.go

Co-authored-by: Steven Soroka <[email protected]>

sjwang90 · 2020-08-24T21:20:02Z

@fdamador if you're interesting in testing this code with authentication options based off your original PR that would be really helpful.

This reverts commit 2f734bd.

…pcua-input

…put, updated self-signed cert organization.

plugins/inputs/opcua/README.md

sjwang90 · 2020-08-27T22:37:59Z

plugins/inputs/opcua/README.md

+## Guide:
+## An OPC UA node ID may resemble: "n=3,s=Temperature"
+## In this example, n=3 is indicating the namespace is '3'.
+## s=Temperature is indicting that the identifier type is a 'string' and the indentifier value is 'Temperature'
+## This temperature node may have a current value of 79.0, which would possibly make the value a 'float'.
+## To gather data from this node you would need to enter the following line into 'nodes' property above:
+##     {name="SomeLabel", namespace="3", identifier_type="s", identifier="Temperature", data_type="float", description="Some description."},


Does this need to be in the sample config? Could bring it out as just plain text in the README

That sounds clearer to me too. Maybe make a subsection under configuration to describe how to use the nodes setting. See https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md

sjwang90 · 2020-08-27T22:39:38Z

plugins/inputs/opcua/README.md

@@ -0,0 +1,76 @@
+# Telegraf Input Plugin: opcua_client
+
+The opcua_client plugin retrieves data from OPCUA slave devices


change "slave devices" to "client devices"

reimda · 2020-09-01T20:58:24Z

plugins/inputs/opcua/opcua_client_test.go

+	var err error
+
+	o.Name = "testing"
+	o.Endpoint = "opc.tcp://opcua.rocks:4840"


We can't depend on an internet test service in unit tests because the tests run in CI and need to be reliable. Either mock the endpoint you need for the test or disable the test in the short test set used in CI.

if testing.Short() { t.Skip("Skipping integration test in short mode") }

reimda · 2020-09-01T21:06:52Z

plugins/inputs/opcua/README.md

+
+```toml
+
+# ## Connection Configuration


Clean up the comments in the example config here and in sampleConfig. They need to match the commenting and indentation style because they are used when generating a config. See https://github.com/influxdata/telegraf/wiki/SampleConfig

Accurate default values should be included here and commented out with a single hash. Documentation is commented out with two hashes, not hash space hash hash

plugins/inputs/opcua/README.md

reimda · 2020-09-01T21:15:34Z

plugins/inputs/opcua/opcua_client.go

+	Interval       string   `toml:"time_interval"`
+	TimeOut        int      `toml:"timeout"`


Did you look into this? It seems error prone to allow people to set the interval as a string. The docs say 5s or 10s which is the format config.Duration uses.

reimda · 2020-09-01T21:23:41Z

plugins/inputs/opcua/README.md

+timeout = 30
+#
+#  # Time Inteval, default = 10s
+time_interval = "5s"


The code in opcua_client.go says time_interval defaults to opcua.DefaultSubscriptionInterval. The gopcua docs say that's 100ms. https://godoc.org/github.com/gopcua/opcua#pkg-constants

Which default is right? 10s, 5s, 100ms?

reimda · 2020-09-01T21:31:57Z

plugins/inputs/opcua/README.md

+## Guide:
+## An OPC UA node ID may resemble: "n=3,s=Temperature"
+## In this example, n=3 is indicating the namespace is '3'.
+## s=Temperature is indicting that the identifier type is a 'string' and the indentifier value is 'Temperature'
+## This temperature node may have a current value of 79.0, which would possibly make the value a 'float'.
+## To gather data from this node you would need to enter the following line into 'nodes' property above:
+##     {name="SomeLabel", namespace="3", identifier_type="s", identifier="Temperature", data_type="float", description="Some description."},


That sounds clearer to me too. Maybe make a subsection under configuration to describe how to use the nodes setting. See https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md

reimda · 2020-09-01T21:33:58Z

plugins/inputs/opcua/README.md

+nodes = [
+		{name="ProductName", namespace="0", identifier_type="i", identifier="2261", data_type="string", description="open62541 OPC UA Server"},
+		{name="ProductUri", namespace="0", identifier_type="i", identifier="2262", data_type="string", description="http://open62541.org"},
+		{name="ManufacturerName", namespace="0", identifier_type="i", identifier="2263", data_type="string", description="open62541"},
+]


Move this out of the sample config. It's a good config example and it makes sense to have it in the readme but we don't want to include it in everyone's config.

plugins/inputs/opcua/README.md

reimda · 2020-09-01T21:40:43Z

plugins/inputs/opcua/README.md

+#  # Security policy: None, Basic128Rsa15, Basic256, Basic256Sha256. Default: auto
+security_policy = "None"
+#
+#  # Security mode: None, Sign, SignAndEncrypt. Default: auto
+security_mode = "None"
+#


Include auto in the list of valid values. Don't set them to "None" in the sample config if they default to auto.

Also, where in the code do these default to auto? I didn't find it in a quick review.

reimda · 2020-09-01T21:42:28Z

plugins/inputs/opcua/README.md

+#  # password = "mypassword"
+#
+#  ## Measurements
+#  ## node id to subscribe to


Use full sentences with punctuation and capitalization.

chrishayles added 6 commits August 18, 2020 23:12

Added placeholder and common files for opcua input.

36cda2c

added input code

610087b

Added opcua input plugin

8bf22b6

Removed unnesessary files, properties, and comments. Updated README.

f89e30f

Removed mystery files

925887c

Added to LICENSE_OF_DEPENDENCIES.md

32ed4de

ssoroka suggested changes Aug 20, 2020

View reviewed changes

Update plugins/inputs/opcua/opcua_client.go

82f20c4

Co-authored-by: Steven Soroka <[email protected]>

sjwang90 added the new plugin label Aug 21, 2020

ssoroka added the area/opcua label Aug 21, 2020

chrishayles added 5 commits August 24, 2020 15:02

updated

2f734bd

Revert "updated"

814d554

This reverts commit 2f734bd.

Merge branch 'opcua-input' of github.com:haylesnortal/telegraf into o…

140873c

…pcua-input

Updated gopuca library version to resolve error in ProSys Simulator v5.

26c3c6b

Removed builder, updated tags and fields, removed unnecessary log out…

8ce0858

…put, updated self-signed cert organization.

sjwang90 reviewed Aug 27, 2020

View reviewed changes

reimda suggested changes Sep 1, 2020

View reviewed changes

reimda and others added 2 commits September 2, 2020 14:59

Merge branch 'master' into opcua-input

0bb927b

go mod tidy

bb1055b

reimda merged commit 160e1d1 into influxdata:master Sep 2, 2020

sjwang90 mentioned this pull request Sep 8, 2020

New opc ua client plugin #7213

Closed

3 tasks

idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020

Add OPC UA input plugin (influxdata#8009)

572085d

arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023

Add OPC UA input plugin (influxdata#8009)

a6d4e78

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Opcua input plugin + authentication options #8009

Opcua input plugin + authentication options #8009

chrishayles commented Aug 20, 2020

ssoroka left a comment

ssoroka Aug 20, 2020

ssoroka Aug 20, 2020

chrishayles Aug 20, 2020

reimda Sep 1, 2020

sjwang90 commented Aug 24, 2020

sjwang90 Aug 27, 2020

reimda Sep 1, 2020

sjwang90 Aug 27, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

reimda Sep 1, 2020

		Interval string `toml:"time_interval"`
		TimeOut int `toml:"timeout"`

		@@ -0,0 +1,76 @@
		# Telegraf Input Plugin: opcua_client

		The opcua_client plugin retrieves data from OPCUA slave devices

Opcua input plugin + authentication options #8009

Opcua input plugin + authentication options #8009

Conversation

chrishayles commented Aug 20, 2020

Required for all PRs:

ssoroka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjwang90 commented Aug 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment