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

feat: add opentelemetry variables #8834

Closed
wants to merge 0 commits into from
Closed

Conversation

lework
Copy link
Contributor

@lework lework commented Feb 10, 2023

Description

Span information recorded in the access log.

Fixes # #8765

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@@ -333,6 +333,12 @@ function _M.rewrite(conf, api_ctx)
kind = span_kind.server,
attributes = attributes,
})

local span_context = ctx.sp:context()
Copy link
Contributor

Choose a reason for hiding this comment

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

use ctx:span():context()

@@ -333,6 +333,12 @@ function _M.rewrite(conf, api_ctx)
kind = span_kind.server,
attributes = attributes,
})

local span_context = ctx.sp:context()
ngx.var.opentelemetry_context_traceparent = string.format("00-%s-%s-%02x",span_context.trace_id, span_context.span_id, span_context.trace_flags)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing space before span_context.trace_id

local span_context = ctx.sp:context()
ngx.var.opentelemetry_context_traceparent = string.format("00-%s-%s-%02x",span_context.trace_id, span_context.span_id, span_context.trace_flags)
ngx.var.opentelemetry_trace_id = span_context.trace_id
ngx.var.opentelemetry_span_id = span_context.span_id
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a bool field set_ngx_var(default false) in opentelemetry attr_schema, so user can enable/disable it, and keep backward compatibility.

@@ -89,6 +89,31 @@ plugin_attr:
max_export_batch_size: 2
```

## Variables

The following nginx variables are set by the opentelemetry:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The following nginx variables are set by the opentelemetry:
The following nginx variables are set by OpenTelemetry:

- `opentelemetry_trace_id` - Trace Id of the current span
- `opentelemetry_span_id` - Span Id of the current span

How to use variables ?you have to add it to your configuration file (`conf/config.yaml`):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
How to use variables ?you have to add it to your configuration file (`conf/config.yaml`):
How to use variables? You have to add it to your configuration file (`conf/config.yaml`):

@lework
Copy link
Contributor Author

lework commented Feb 13, 2023

@yangxikun @shreemaan-abhishek Thank you for the suggestion, has completed the revision.

@lework
Copy link
Contributor Author

lework commented Feb 16, 2023

I synced the fork, which caused this pr to close. I brought up a new pr #8871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants