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

in_opentelemetry: attempt to fix tag_from_uri #8881

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

shaohme
Copy link
Contributor

@shaohme shaohme commented May 27, 2024

Pass on tag with length so that tag_from_uri get applied on OpenTelemetry input plugin

Fixes #8734

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
service:
  flush: 1
  log_file: /tmp/fluent-bit.log
  daemon: false
  log_level: debug
pipeline:
  inputs:
    - name: opentelemetry
      listen: 127.0.0.1
      port: 9999
      raw_traces: false
      successful_response_code: 200
      tag_from_uri: true
      http2: false
  outputs:
    - name: stdout
      match: "v1_traces"
  • Debug log output from testing the change
[2024/05/27 14:58:22] [ info] [fluent bit] version=3.0.7, commit=dffcd42ada, pid=1820178
[2024/05/27 14:58:22] [debug] [engine] coroutine stack size: 24576 bytes (24.0K)
[2024/05/27 14:58:22] [ info] [storage] ver=1.5.2, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2024/05/27 14:58:22] [ info] [cmetrics] version=0.9.0
[2024/05/27 14:58:22] [ info] [ctraces ] version=0.5.1
[2024/05/27 14:58:22] [ info] [input:opentelemetry:opentelemetry.0] initializing
[2024/05/27 14:58:22] [ info] [input:opentelemetry:opentelemetry.0] storage_strategy='memory' (memory only)
[2024/05/27 14:58:22] [debug] [opentelemetry:opentelemetry.0] created event channels: read=22 write=18
[2024/05/27 14:58:22] [debug] [downstream] listening on 127.0.0.1:9999
[2024/05/27 14:58:22] [ info] [input:opentelemetry:opentelemetry.0] listening on 127.0.0.1:9999
[2024/05/27 14:58:22] [debug] [stdout:stdout.0] created event channels: read=24 write=25
[2024/05/27 14:58:22] [ info] [sp] stream processor started
[2024/05/27 14:58:22] [ info] [output:stdout:stdout.0] worker #0 started
[2024/05/27 14:58:24] [debug] [task] created task=0x7f2811436640 id=0 OK
[2024/05/27 14:58:24] [debug] [output:stdout:stdout.0] task_id=0 assigned to thread #0
[2024/05/27 14:58:24] [debug] [output:stdout:stdout.0] ctr decode msgpack returned : 6
[2024/05/27 14:58:24] [debug] [out flush] cb_destroy coro_id=0
[2024/05/27 14:58:24] [debug] [task] destroy task=0x7f2811436640 (task_id=0)
[2024/05/27 14:58:25] [debug] [task] created task=0x7f28114366e0 id=0 without routes, dropping.
[2024/05/27 14:58:25] [debug] [task] destroy task=0x7f28114366e0 (task_id=0)
[2024/05/27 14:58:25] [debug] [task] created task=0x7f28114366e0 id=0 OK
[2024/05/27 14:58:25] [debug] [output:stdout:stdout.0] task_id=0 assigned to thread #0
[2024/05/27 14:58:25] [debug] [output:stdout:stdout.0] ctr decode msgpack returned : 6
[2024/05/27 14:58:25] [debug] [out flush] cb_destroy coro_id=1
[2024/05/27 14:58:25] [debug] [task] destroy task=0x7f28114366e0 (task_id=0)
[2024/05/27 14:58:26] [debug] [task] created task=0x7f28114365a0 id=0 without routes, dropping.
[2024/05/27 14:58:26] [debug] [task] destroy task=0x7f28114365a0 (task_id=0)
[2024/05/27 14:58:26] [debug] [task] created task=0x7f28114365a0 id=0 OK
[2024/05/27 14:58:26] [debug] [output:stdout:stdout.0] task_id=0 assigned to thread #0
[2024/05/27 14:58:26] [debug] [output:stdout:stdout.0] ctr decode msgpack returned : 6
[2024/05/27 14:58:26] [debug] [out flush] cb_destroy coro_id=2
[2024/05/27 14:58:26] [debug] [task] destroy task=0x7f28114365a0 (task_id=0)
[2024/05/27 14:58:28] [ warn] [engine] service will shutdown in max 5 seconds
[2024/05/27 14:58:28] [ info] [engine] service has stopped (0 pending tasks)
[2024/05/27 14:58:28] [ info] [output:stdout:stdout.0] thread worker #0 stopping...
[2024/05/27 14:58:28] [ info] [output:stdout:stdout.0] thread worker #0 stopped
  • [N/A] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@nuclearpidgeon
Copy link
Contributor

I'm at a loss as to why this was never implemented in the first place... perhaps at some point there was an issue with tag-based routing at the point this code was written ¯_(ツ)_/¯. Nice work fixing it up! This was what I was hoping to PR at some point myself.

@cosmo0920
Copy link
Contributor

Hi, fluent organization requires to add DCO. So, could you add Signed-off in your commit?
The description is here: https://github.com/fluent/fluent-bit/pull/8881/checks?check_run_id=25458914982

@shaohme shaohme force-pushed the fix-in-otel-tag-from-uri branch from 591b05a to 3d84849 Compare May 28, 2024 04:02
@shaohme
Copy link
Contributor Author

shaohme commented May 28, 2024

Hi, fluent organization requires to add DCO. So, could you add Signed-off in your commit? The description is here: https://github.com/fluent/fluent-bit/pull/8881/checks?check_run_id=25458914982

Sure. Should be added now. Had to force push.

@cosmo0920
Copy link
Contributor

cosmo0920 commented May 28, 2024

Plus, we need to add prefix for changed module name into commit message(s).
In this case, we need to use "in_opentelemery: attempt to fix tag_from_uri" in your commit message.

see also: https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md#commit-changes

@shaohme
Copy link
Contributor Author

shaohme commented May 28, 2024

Plus, we need to add prefix for changed module name into commit message(s). In this case, we need to use "in_opentelemery: attempt to fix tag_from_uri" in your commit message.

see also: https://github.com/fluent/fluent-bit/blob/master/CONTRIBUTING.md#commit-changes

Should be fixed now. And I should probably have read a contributors manual first ... 😏

@@ -1736,6 +1741,8 @@ int opentelemetry_prot_handle(struct flb_opentelemetry *ctx, struct http_conn *c
}
}

size_t tag_len = flb_sds_len(tag);
Copy link
Contributor

Choose a reason for hiding this comment

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

Declaring middle of the function is not good coding style on Fluent Bit.
Could you declare a variable in the beginning of this function? And then, calculate length of tag here should be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Declaring middle of the function is not good coding style on Fluent Bit. Could you declare a variable in the beginning of this function? And then, calculate length of tag here should be fine.

Sure can. Should be pushed now.

@cosmo0920 cosmo0920 changed the title attempt to fix tag_from_uri in_opentelemetry: attempt to fix tag_from_uri May 28, 2024
@shaohme shaohme force-pushed the fix-in-otel-tag-from-uri branch from 7d658e7 to 4312c7e Compare May 28, 2024 10:32
Copy link
Contributor

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@edsiper edsiper added this to the Fluent Bit v3.1.0 milestone Jun 7, 2024
@edsiper edsiper merged commit 477f656 into fluent:master Jun 13, 2024
44 checks passed
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.

tag_from_uri in opentelemetry input plugin has no effect
4 participants