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 tracing instrumentation release notes #3477

Merged
merged 2 commits into from
Dec 1, 2023

Conversation

ka3de
Copy link
Contributor

@ka3de ka3de commented Nov 29, 2023

What?

Adds the release notes corresponding to the base tracing instrumentation implemented in #3445.

Why?

In order to reflect this new feature in the upcoming v0.48.0 release notes.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes. (N/A)
  • I have run linter locally (make lint) and all checks pass. (N/A)
  • I have run tests locally (make tests) and all tests pass. (N/A)
  • I have commented on my code, particularly in hard-to-understand areas. (N/A)

@ka3de ka3de self-assigned this Nov 29, 2023
release notes/v0.48.0.md Outdated Show resolved Hide resolved
@ka3de ka3de marked this pull request as ready for review November 29, 2023 13:56
release notes/v0.48.0.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 30, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (release-notes-v0.48.0@3b54410). Click here to learn what that means.

❗ Current head 24870e4 differs from pull request most recent head feaa3a0. Consider uploading reports for the commit feaa3a0 to get more accurate results

Additional details and impacted files
@@                   Coverage Diff                    @@
##             release-notes-v0.48.0    #3477   +/-   ##
========================================================
  Coverage                         ?   73.28%           
========================================================
  Files                            ?      264           
  Lines                            ?    19960           
  Branches                         ?        0           
========================================================
  Hits                             ?    14628           
  Misses                           ?     4418           
  Partials                         ?      914           
Flag Coverage Δ
ubuntu 73.28% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +172 to +179
Where `opt`s can be one of the following options:
* `proto`: Specifies the protocol to use in the connection to the traces backend. Supports `grpc` *(default)* and `http`.
* `header.<header_name>`: Specifies an additional header to include in the connection to the traces backend.

Example:
```
K6_TRACES_OUTPUT=https://traces.k6.io/v1/traces,proto=http,header.Authorization=Bearer token
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to replace it with just a link to the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we want to link to the documentation, then I would say we should remove the last part since:

The format for the otel traces output configuration is the following:
...

Otherwise mentioning the format without mentioning the details in the release notes might not be the best. In any case, for me it's always tricky to decide what is included in the release notes and what is left for documentation. In this case I think the content is not so long so it can be included directly in the release notes, without requiring our users to go to the docs, but no strong opinion.

Copy link
Contributor

@olegbespalov olegbespalov left a comment

Choose a reason for hiding this comment

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

Let's merge this, and we could give a final round of the review in the #3458

@ka3de ka3de merged commit e890ae3 into release-notes-v0.48.0 Dec 1, 2023
19 of 21 checks passed
@ka3de ka3de deleted the add/tracing-release-notes branch December 1, 2023 07:49
@olegbespalov
Copy link
Contributor

olegbespalov commented Dec 1, 2023

@ka3de not sure how actively we should promote this option, but at least shortly, maybe we need to mention a PR for the k6-docs. See an example grafana/k6-docs#1417

@mstoykov @codebien what do you think?

@ka3de
Copy link
Contributor Author

ka3de commented Dec 1, 2023

@ka3de not sure how actively we should promote this option, but at least shortly, maybe we need to mention a PR for the k6-docs.

There is a PR already, see grafana/k6-docs#1433. I'm waiting for some more reviews, but otherwise I will merge it on Monday morning max.

@olegbespalov
Copy link
Contributor

My bad, missed that 👍

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.

5 participants