feat(opentelemetry): support variable resource attributes #13839

SuzyWangIBMer · 2024-11-06T19:40:50Z

Summary

This change allows render resource attributes as lua code.
For example,

  config:
    traces_endpoint: "https://otlp-red-saas.instana.io:4318/v1/traces"
    resource_attributes:
      host.id: "$(headers.host)"

host.id is 0.0.0.0:9000 or depends on headers.host value.

Checklist

The Pull Request has tests
A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix #12538

samugi · 2024-11-15T09:00:03Z

failing test is expected, let's rebase after #13877 is merged

samugi

Thank you so much for your contribution, this is great! I left a few comments for things that I feel could be improved a bit. There's some code duplication with request-transformer that we may want to take care of, but that refactor can be done later.

kong/plugins/opentelemetry/utils.lua

kong/plugins/opentelemetry/traces.lua

kong/plugins/opentelemetry/logs.lua

kong/plugins/opentelemetry/handler.lua

kong/plugins/opentelemetry/utils.lua

spec/03-plugins/37-opentelemetry/02-schema_spec.lua

kong/plugins/opentelemetry/handler.lua

spec/03-plugins/37-opentelemetry/02-schema_spec.lua

kong/plugins/opentelemetry/utils.lua

samugi

thanks for the quick action! I left a couple more suggestions, then I think it's good to go.

kong/plugins/opentelemetry/handler.lua

kong/plugins/opentelemetry/logs.lua

kong/plugins/opentelemetry/traces.lua

kong/plugins/opentelemetry/utils.lua

kong/plugins/opentelemetry/handler.lua

samugi

Looks good to me, thank you!!

Could you just update the documentation line for this configuration field in the plugin's schema? This is used to auto-generate docs.

samugi

great job and thanks again!

Edit: sorry about the failing CI, it's because the new schema description exceeded the length limit, if you undo that change I'll take care of updating the plugin's docs as required.

SuzyWangIBMer · 2024-12-04T16:24:30Z

Do we need a second reviewer to get this in?

samugi · 2024-12-05T07:23:35Z

yes @SuzyWangIBMer I'm trying to get the second approval, thank you for your patience

kong/plugins/opentelemetry/utils.lua

fffonion · 2024-12-05T07:40:52Z

kong/plugins/opentelemetry/utils.lua

 return {
  http_export_request = http_export_request,
  get_headers = get_headers,
  _log_prefix = _log_prefix,
+  read_resource_attributes = read_resource_attributes,


should we have a unit test for this function?

We now have a test for possible resource attributes sent and received. I can also work on one if you think it does not cover everything.

kong/spec/03-plugins/37-opentelemetry/04-exporter_spec.lua

Lines 566 to 572 in 982f2a3

setup_instrumentations("all", {

resource_attributes = {

["service.name"] = "kong_oss",

["os.version"] = "debian",

["host.name"] = "$(headers.host)",

["validstr"] = "$($@#)",

}

@SuzyWangIBMer I think @fffonion is referring to the fact we could have a dedicated unit test for this function, in addition to the integration test you've mentioned, for example in a spec/03-plugins/37-opentelemetry/06-utils_spec.lua file.

Sure, I will work on one.

PR updated, let me know what do you guys think.

kong/plugins/opentelemetry/utils.lua

kong/plugins/opentelemetry/handler.lua

spec/03-plugins/37-opentelemetry/04-exporter_spec.lua

spec/03-plugins/37-opentelemetry/07-utils_spec.lua

nowNick

That looks great! Thank you @SuzyWangIBMer for your contribution! 🚀

samugi

@SuzyWangIBMer please address these, then CI should become green and we should be ready.

samugi · 2024-12-21T10:38:23Z

kong/plugins/opentelemetry/utils.lua

@@ -1,9 +1,17 @@
 local http = require "resty.http"
 local clone = require "table.clone"
-
+local sandbox = require "kong.tools.sandbox"
+local deep_copy = require("kong.tools.table").deep_copy


Suggested change

local deep_copy = require("kong.tools.table").deep_copy

local cycle_aware_deep_copy = require("kong.tools.table").cycle_aware_deep_copy

sorry, I noticed this last-minute

samugi · 2024-12-21T10:38:43Z

kong/plugins/opentelemetry/utils.lua

+  local template_env = {}
+  if lua_enabled and sandbox_enabled then
+    -- load the sandbox environment to be used to render the template
+    template_env = deep_copy(sandbox.configuration.environment)


Suggested change

template_env = deep_copy(sandbox.configuration.environment)

template_env = cycle_aware_deep_copy(sandbox.configuration.environment)

samugi · 2024-12-21T10:39:49Z

spec/03-plugins/37-opentelemetry/07-utils_spec.lua

@@ -0,0 +1,52 @@
+local LOG_PHASE = require("kong.pdk.private.phases").phases.log
+


Suggested change

require "spec.helpers"

we still need to require helpers because it does some needed setup on the environment, but don't assign it to a local variable, or else the linter will complain because it's unused.

pull-request-size bot added the size/L label Nov 6, 2024

github-actions bot added plugins/opentelemetry schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Nov 6, 2024

team-eng-enablement added the author/community PRs from the open-source community (not Kong Inc) label Nov 6, 2024

samugi self-assigned this Nov 15, 2024

samugi force-pushed the feat/otel_variable_resource_attributes branch from 6dc12b9 to eaf8f25 Compare November 15, 2024 13:31

samugi self-requested a review November 15, 2024 17:07

samugi requested changes Nov 18, 2024

View reviewed changes

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from eaf8f25 to 8a185af Compare November 18, 2024 22:21

SuzyWangIBMer requested a review from samugi November 20, 2024 13:34

samugi reviewed Nov 20, 2024

View reviewed changes

kong/plugins/opentelemetry/handler.lua Outdated Show resolved Hide resolved

spec/03-plugins/37-opentelemetry/02-schema_spec.lua Show resolved Hide resolved

kong/plugins/opentelemetry/utils.lua Outdated Show resolved Hide resolved

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch 2 times, most recently from 0581567 to 4cfc804 Compare November 21, 2024 19:49

SuzyWangIBMer requested a review from samugi November 21, 2024 19:50

samugi reviewed Nov 22, 2024

View reviewed changes

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from 4cfc804 to aba7105 Compare November 22, 2024 15:31

SuzyWangIBMer requested a review from samugi November 22, 2024 16:12

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from aba7105 to 48c4be0 Compare November 25, 2024 18:36

samugi reviewed Nov 28, 2024

View reviewed changes

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch 2 times, most recently from f9e3afb to 4e2c7ab Compare November 28, 2024 17:11

samugi approved these changes Nov 28, 2024

View reviewed changes

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from 4e2c7ab to d5a0c23 Compare November 28, 2024 18:02

SuzyWangIBMer requested a review from samugi December 2, 2024 13:34

samugi approved these changes Dec 3, 2024

View reviewed changes

fffonion reviewed Dec 5, 2024

View reviewed changes

windmgc reviewed Dec 5, 2024

View reviewed changes

spec/03-plugins/37-opentelemetry/04-exporter_spec.lua Outdated Show resolved Hide resolved

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from d5a0c23 to 982f2a3 Compare December 5, 2024 22:11

SuzyWangIBMer requested review from windmgc and fffonion December 9, 2024 13:07

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from 982f2a3 to 7239a60 Compare December 13, 2024 03:21

samugi reviewed Dec 17, 2024

View reviewed changes

spec/03-plugins/37-opentelemetry/07-utils_spec.lua Outdated Show resolved Hide resolved

spec/03-plugins/37-opentelemetry/07-utils_spec.lua Outdated Show resolved Hide resolved

SuzyWangIBMer added 2 commits December 17, 2024 09:44

feat(opentelemetry):set variable resource attributes

a9d78d5

test(opentelemetry): support variable resource attributes

f56f481

SuzyWangIBMer force-pushed the feat/otel_variable_resource_attributes branch from 7239a60 to f56f481 Compare December 17, 2024 17:45

samugi requested a review from nowNick December 18, 2024 08:22

nowNick approved these changes Dec 20, 2024

View reviewed changes

samugi requested changes Dec 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(opentelemetry): support variable resource attributes #13839

feat(opentelemetry): support variable resource attributes #13839

SuzyWangIBMer commented Nov 6, 2024 •

edited

Loading

samugi commented Nov 15, 2024

samugi left a comment

samugi left a comment

samugi left a comment

samugi left a comment •

edited

Loading

SuzyWangIBMer commented Dec 4, 2024

samugi commented Dec 5, 2024

fffonion Dec 5, 2024

SuzyWangIBMer Dec 5, 2024

samugi Dec 9, 2024

SuzyWangIBMer Dec 9, 2024

SuzyWangIBMer Dec 13, 2024

nowNick left a comment

samugi left a comment

samugi Dec 21, 2024

samugi Dec 21, 2024

samugi Dec 21, 2024

	setup_instrumentations("all", {
	resource_attributes = {
	["service.name"] = "kong_oss",
	["os.version"] = "debian",
	["host.name"] = "$(headers.host)",
	["validstr"] = "$($@#)",
	}

	local deep_copy = require("kong.tools.table").deep_copy
	local cycle_aware_deep_copy = require("kong.tools.table").cycle_aware_deep_copy

	template_env = deep_copy(sandbox.configuration.environment)
	template_env = cycle_aware_deep_copy(sandbox.configuration.environment)

		@@ -0,0 +1,52 @@
		local LOG_PHASE = require("kong.pdk.private.phases").phases.log

feat(opentelemetry): support variable resource attributes #13839

Are you sure you want to change the base?

feat(opentelemetry): support variable resource attributes #13839

Conversation

SuzyWangIBMer commented Nov 6, 2024 • edited Loading

Summary

Checklist

Issue reference

samugi commented Nov 15, 2024

samugi left a comment

Choose a reason for hiding this comment

samugi left a comment

Choose a reason for hiding this comment

samugi left a comment

Choose a reason for hiding this comment

samugi left a comment • edited Loading

Choose a reason for hiding this comment

SuzyWangIBMer commented Dec 4, 2024

samugi commented Dec 5, 2024

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

nowNick left a comment

Choose a reason for hiding this comment

samugi 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

SuzyWangIBMer commented Nov 6, 2024 •

edited

Loading

samugi left a comment •

edited

Loading